diff --git a/.agents/skills/libreqos_security_audit/SKILL.md b/.agents/skills/libreqos_security_audit/SKILL.md new file mode 100644 index 00000000..3261d7b9 --- /dev/null +++ b/.agents/skills/libreqos_security_audit/SKILL.md @@ -0,0 +1,457 @@ +--- +name: libreqos_security_audit +description: Repo-local LibreQoS workflow for release security audit passes. Use when auditing LibreQoS with cargo audit, cargo machete, cargo tree, CVE triage, network control-plane exposure review, bridged-interface/eBPF malformed-traffic review, panic/error-handling/type-loss review, node_manager privacy/auth/XSS review, executive-summary/conclusion grading, and audit-file findings updates. +--- + +# LibreQoS Security Audit + +Use this skill for recurring LibreQoS security audit passes in this repo. It +covers the Rust dependency baseline, network control-plane exposure review, +bridged-interface/eBPF malformed-traffic review, and panic/error-handling/type +loss review, node_manager privacy/auth/XSS review, and final executive-summary +wrap-up. Use additional focused checks for Python, packaging, live-host +configuration, secrets, and +authentication flows outside these scopes. + +## Scope + +- Keep the audit repo-local. Do not install or update global skills. +- Start from the current repo checkout and respect dirty worktree boundaries. +- Write results into the requested audit file. If no file is named, ask before creating one. +- Distinguish security findings from maintenance warnings. "Not maintained" is not a security issue by itself. +- Distinguish reachable vulnerabilities from unused-feature or irrelevant-feature advisories. A vulnerability in a feature LibreQoS does not use is not a security finding by itself. +- Treat an open listener as evidence, not a vulnerability by itself. Record a + security finding only when the listener exposes exploitability, credential or + session leakage, unauthenticated access beyond the intended demo/public mode, + authorization bypass, brute-force exposure, remotely triggerable panic/DoS, or + a control action reachable without the expected protection. + +## Step 1: Rust Dependency Audit + +Run from `src/rust/` unless the user asks for a narrower crate: + +```text +cargo audit +cargo machete +cargo tree --locked --workspace --depth 1 +``` + +Use the depth-1 tree only for a quick workspace dependency summary. For every +advisory or suspicious dependency, run an inverse tree query for the exact crate +and version when Cargo can resolve it, such as `cargo tree -i rand --locked` or +`cargo tree -i rand@0.8.5 --locked`. If Cargo cannot resolve the package ID, +use `cargo audit`'s dependency tree plus `rg` searches through manifests and +source files. + +If sandboxing blocks Cargo registry or advisory-db access, rerun the blocked command with approval instead of switching to stale data. + +### Reachability Rubric + +Treat an advisory as a security finding when at least one of these is true: + +- LibreQoS directly imports the vulnerable crate and calls the affected API. +- A current workspace crate exposes the vulnerable behavior through a LibreQoS + runtime surface: + `lqosd` node-manager HTTP/websocket handlers, `lqos_bus` message handling, + `lqos_config` parsing for `network.json` / `ShapedDevices.csv` / config + files, `lqos_network_devices` runtime access to shaped-device and topology + state, `lqos_topology` / `lqos_topology_compile` topology projection, + `lqos_probe` active probing, `lqos_overrides`, `uisp_integration`, + `lqos_python`, `lqos_setup`, `lqos_netplan_helper`, `lqos_sys` + TC/XDP/eBPF interaction, queue/bakery crates, or support-tool archive/input + handling. +- The advisory applies to default features or enabled features in + `Cargo.toml` / `Cargo.lock`, and no code-level usage check is needed for the + vulnerable behavior to be present. + +Expected evidence searches include the affected crate name in `Cargo.toml` +files, affected function/type names in `src/rust/**`, relevant feature names in +manifests and `Cargo.lock`, and logger/auth/network/file-parsing entry points +when the advisory depends on runtime conditions. + +Treat an advisory as not currently security-relevant when all of these are true: + +- The affected crate is present only through an unused optional feature, + build-only path, test-only path, or API surface LibreQoS does not call. +- Feature flags and source search support that conclusion. +- The audit note records what was checked. + +If the evidence is incomplete, record it as "reachability unknown" and list the +specific follow-up needed. Do not call it safe. + +### Triage Rules + +For every `cargo audit` result: + +- Record CVE/RUSTSEC/GHSA security advisories as findings only when they meet the reachability rubric above. +- For each security finding, include: + - the advisory ID + - the repo-relative path to the `Cargo.toml` that imports the vulnerable crate + - repo-relative source paths that call the affected API or make the dependency reachable, when applicable + - a short vulnerability description + - recommended actions +- Do not list maintenance-only advisories as security findings. Put them in a separate non-security notes section. +- If an advisory depends on specific feature flags, runtime configuration, or API calls, verify those paths with `rg` before writing the conclusion. +- If reachability is unclear, use "reachability unknown" and say exactly what was checked and what remains unknown. + +For `cargo machete` results: + +- Record unused dependencies separately from security findings. +- Include the repo-relative manifest path and dependency name. +- Recommend removal plus the smallest focused validation command. Use a real + crate name in the audit note, for example `cargo check -p + lqos_network_devices`. + +For `cargo tree`: + +- Use depth-1 output only for a quick workspace summary. +- Use inverse tree queries to identify direct and transitive import paths for findings. +- Keep dependency-surface summaries short and tied to an audit decision. + +### Audit Section Checklist + +The audit-file section must include: + +- Heading: `Rust dependency audit`. +- Date, scope, and exact commands run. +- Summary bullets covering dependency count, security findings, unused + dependencies, and non-security warnings. +- One subsection per security finding with advisory ID, affected crate, import + paths, source-use paths where applicable, short description, reachability + decision, and recommended actions. +- A separate unused-dependencies subsection for `cargo machete` output. +- A separate non-security warnings subsection for unmaintained or informational + advisories. +- No placeholders, no `...`, and no unresolved `` tokens. + +## Step 2: Network Control-Plane Exposure Audit + +Use this step when the audit turns to external threats over the LibreQoS control +plane. + +### Scope Assumptions + +- Assume the operator installed the Caddy / SSL / TLS option. +- Treat Linux, Ubuntu, kernel, and distribution package vulnerabilities as out + of scope because LibreQoS cannot fix them in-repo. +- Treat the control interface as in scope. Anything listening or reachable on + the control interface should be reviewed. +- Treat the two bridge interfaces, whether XDP or Linux bridge backed by eBPF, + as out of scope for this section. +- Include the sibling `../../lqos_api/` repo as read-only audit context when it + is present, because the API is exposed behind Caddy. +- Do not edit `../../lqos_api/` unless the user explicitly authorizes + cross-repo changes. + +### Evidence To Gather + +Review these surfaces first: + +```text +src/rust/lqos_setup/src/ssl.rs +src/rust/lqos_setup/src/web.rs +src/rust/lqosd/src/node_manager/ +docs/v2.0/https-caddy.md +docs/v2.0/api.md +../../lqos_api/src/ +../../lqos_api/README.md +``` + +Use `rg` searches for listener addresses, routes, middleware, auth checks, +cookie settings, CORS, Caddy upstreams, and panic-prone request handling: + +```text +rg "bind\\(|TcpListener|listen|reverse_proxy|Caddy|CorsLayer|very_permissive|allow_anonymous|SameSite|Cookie|x-bearer|route_layer|unwrap\\(" src/rust docs ../../lqos_api +``` + +For each reachable service or route, identify: + +- listener address and port +- whether Caddy proxies it and whether the direct port remains reachable +- authentication and authorization mechanism +- unauthenticated routes and whether they expose control, data, or only health/docs +- state-changing routes and their protection +- cookie flags, CORS policy, CSRF/origin checks, and session behavior +- rate limits or backoff for login/API authentication attempts +- request paths where malformed unauthenticated input can panic + +### Triage Rules + +Count these as likely security findings when evidence supports them: + +- a direct control-plane HTTP listener bypasses the expected Caddy/TLS path + for authenticated API traffic +- a route that changes state or exposes sensitive operational data is reachable + without the expected auth, except for the explicit public/demo read-only mode +- an auth or request middleware can panic on unauthenticated remote input +- credentialed CORS, cookie flags, or missing CSRF/origin checks let another + browser origin use an operator session +- login or bearer-token checks lack reasonable throttling for a network-exposed + control-plane service + +Do not count these as findings by themselves: + +- Caddy or LibreQoS listening on a port when the route is protected as intended +- Caddy serving HTTPS for the WebUI/API path +- public API documentation that exposes only endpoint shape and no secret or + state-changing capability +- `allow_anonymous` when the operator intentionally enabled the documented + public/demo read-only mode +- first-run setup exposure when the setup token and lifecycle are being audited + separately, unless the current section finds a concrete bypass + +### Audit Section Checklist + +The audit-file section must include: + +- Heading: `Network control-plane audit`. +- Date, scope assumptions, and exact files or directories reviewed. +- Summary bullets separating findings from observations. +- One subsection per finding with the repo-relative path, short description, + exposure/threat, and recommended actions. +- A separate observations / not-findings subsection for open listeners or public + docs that are intentional and not vulnerable by themselves. +- No placeholders, no `...`, and no unresolved `` tokens. + +## Step 3: Bridged Interface / eBPF Malformed-Traffic Audit + +Use this step when the audit turns to bridged interfaces and the eBPF datapath. +This step is about malformed-packet handling, DoS, map exhaustion, ring-buffer +backpressure, packet-rate debug logging, and userspace handling of eBPF events. +It is not about the control interface. + +Review these BPF-specific surfaces first: + +```text +src/rust/lqos_sys/src/bpf/lqos_kern.c +src/rust/lqos_sys/src/bpf/common/debug.h +src/rust/lqos_sys/src/bpf/common/dissector.h +src/rust/lqos_sys/src/bpf/common/dissector_tc.h +src/rust/lqos_sys/src/bpf/common/flows.h +src/rust/lqos_sys/src/bpf/common/heimdall.h +src/rust/lqos_sys/src/bpf/common/lpm.h +src/rust/lqos_sys/src/bpf/common/throughput.h +src/rust/lqos_sys/src/bpf/common/maximums.h +src/rust/lqos_sys/src/lqos_kernel.rs +src/rust/lqosd/src/throughput_tracker/ +src/rust/lqos_heimdall/src/ +``` + +Use this search as the starting point: + +```text +rg "bpf_debug\\(|frag_off|ihl|tot_len|doff|BPF_MAP_TYPE_HASH|BPF_MAP_TYPE_PERCPU_HASH|BPF_MAP_TYPE_LRU|MAX_FLOWS|MAX_TRACKED_IPS|bpf_ringbuf_output|bpf_probe_read_kernel|data_end|SKB_OVERFLOW|metadata|queue_mapping" src/rust/lqos_sys/src src/rust/lqosd/src/throughput_tracker src/rust/lqos_heimdall/src +``` + +For each packet path, identify the concrete behavior for: + +- malformed Ethernet, VLAN, PPPoE, MPLS, IPv4, IPv6, TCP, UDP, and ICMP input +- IPv4 `ihl`, total length, and fragmentation checks before L4 parsing +- IPv6 extension headers and fragments +- bounded-loop limits for stacked headers and TCP options +- unshaped or spoofed traffic creating pinned-map entries +- map type, max entries, LRU behavior, and insert-failure behavior +- `bpf_trace_printk` / `bpf_debug` calls reachable from bridged traffic +- ring-buffer size checks, backpressure, drop counters, and userspace panics +- metadata paths where malformed packets can become unexpected drops + +Count a finding when malformed, spoofed, or high-cardinality traffic can cause +packet-rate expensive work, non-LRU map exhaustion, bogus flow/RTT/retransmit +state, unexpected packet drops, userspace panic, or unreported loss of capture +events. Do not count verifier-enforced memory safety, unknown non-IP traffic +that merely fails open, or untested live reachability claims as findings by +themselves. + +## Step 4: Panic, Error-Handling, and Type-Loss Audit + +Use this step when the audit turns to code paths that can panic, hide errors, or +silently lose data. Include the sibling `../../lqos_api/src/` when it is present +because it is part of the deployed control-plane surface, but do not edit that +repo unless the user explicitly authorizes cross-repo changes. + +Start with these searches, then inspect only runtime-reachable code. Exclude +tests, fixtures, generated output, vendored bindings, and historical copies such +as `LibreQoS-old.py` unless the user explicitly puts them in scope. + +```text +rg -n "\\bpanic!\\(|\\.unwrap\\(|\\.expect\\(|unreachable!\\(|todo!\\(|unimplemented!\\(|assert!\\(|from_raw_parts|transmute|unsafe \\{|as (u8|u16|u32|usize|i8|i16|i32|f32)|unwrap_or_default\\(|except Exception|except:|pass$" src/rust src --glob '*.py' ../../lqos_api/src +rg -n "as u32|as u16|as f32|partial_cmp\\(.*\\)\\.unwrap|to_str\\(\\)\\.unwrap|parse\\(\\)\\.unwrap|try_into\\(\\)\\.unwrap" src/rust ../../lqos_api/src +rg -n "except Exception|except:|pass$|sys.exit|int\\(|float\\(" src --glob '*.py' --glob '!LibreQoS-old.py' --glob '!LibreQoS-ancient.py' --glob '!LibreQoS.py.new' +``` + +For each candidate, identify: + +- file name and exact line number +- whether the code is request-time, packet-time, config/import-time, startup-only, + test-only, or generated/vendor code +- whether an external user, bridged-interface packet, operator-managed file, or + internal telemetry value can trigger the path +- whether the impact is panic/DoS, poisoned shared state, incorrect rejection, + silent fallback, lossy conversion, wrapped counters, non-finite float handling, + or misleading operational data + +Count a finding when evidence supports one of these: + +- a request, websocket message, packet event, or operator-managed file can panic + a runtime task instead of returning an error +- malformed input can poison or permanently break shared runtime state +- error handling silently continues with a different shaping, auth, or telemetry + result that an operator would not see +- numeric conversion narrows kernel counters, flow counters, bandwidth values, or + timestamps in a way that can wrap, saturate unexpectedly, become non-finite, or + otherwise lose operational data +- an unsafe block reads caller-provided memory without a size check or serializes + uninitialized padding bytes + +Do not count these as findings by themselves: + +- `unwrap` / `expect` in tests, examples, benchmarks, one-shot setup validation, + or process startup where failure stops boot cleanly +- unsafe FFI wrappers that validate sizes and keep pointer lifetimes local +- protocol fields that are intentionally narrower when the code checks range or + logs/clamps loss before export +- broad Python exception handling that only preserves backwards-compatible + tolerance and does not change shaping/auth/security behavior + +The audit-file section must include: + +- Heading: `Panic, error-handling, and type-loss audit`. +- Date, scope, and exact searches or files reviewed. +- Summary bullets separating confirmed findings, reachability-unknown items, and + observations/not-findings. +- One subsection per finding with the repo-relative `path:line`, short + description, exposure/threat, and recommended actions. +- No placeholders, no `...`, and no unresolved `` tokens. + +## Step 5: Node Manager Privacy, Auth, and XSS Audit + +Use this step when reviewing node_manager for missing anonymization of PII, +missing authentication or authorization on data access, browser-storage exposure, +and XSS. Review source files, not generated bundles, unless a generated artifact +is the only shipped source for that behavior. + +Start with these surfaces: + +```text +src/rust/lqosd/src/node_manager/run.rs +src/rust/lqosd/src/node_manager/static_pages.rs +src/rust/lqosd/src/node_manager/auth.rs +src/rust/lqosd/src/node_manager/ws.rs +src/rust/lqosd/src/node_manager/ws/messages.rs +src/rust/lqosd/src/node_manager/local_api/ +src/rust/lqosd/src/node_manager/js_build/src/ +src/rust/lqosd/src/node_manager/static2/template.html +docs/v2.0/node-manager-ui.md +docs/v2.0/components.md +``` + +Use these searches as a starting point: + +```text +rg -n "localStorage|sessionStorage|document\\.cookie|innerHTML|outerHTML|insertAdjacentHTML|eval\\(|Function\\(|onclick=|onerror=|sanitize|DOMPurify|redact|redaction|redactable|allow_anonymous|auth_layer|route_layer|LoginResult|ReadOnly|Admin|Denied" src/rust/lqosd/src/node_manager docs/v2.0 +rg -n "innerHTML\\s*=.*(\\+|`)|simpleRowHtml\\(|href=.*\\+|data-[^=]+=|textContent|innerText" src/rust/lqosd/src/node_manager/js_build/src --glob '*.js' +rg -n "ShapedDevice|network_json|CircuitById|AllShapedDevices|NetworkJson|Search|UnknownIps|CircuitDirectory|device_name|circuit_name|mac|ipv4|ipv6|comment" src/rust/lqosd/src/node_manager +``` + +For each candidate, identify: + +- the exact `path:line` +- whether access is unauthenticated, anonymous read-only, authenticated read-only, + or admin-only +- whether the exposed data includes subscriber/customer identifiers, circuit + names/IDs, device names/IDs, IPs, MACs, comments, topology names, tickets, or + integration secrets +- whether redaction happens server-side, in the transport payload, or only in the + browser display +- whether browser storage persists credentials, session tokens, topology drafts, + dashboard layouts, interface names, VLANs, or other operational data +- whether untrusted strings are inserted with `innerHTML`, HTML tooltips, + attributes, inline handlers, or URLs without escaping and protocol validation + +Count a finding when evidence supports one of these: + +- anonymous/demo/read-only access can retrieve raw PII or sensitive operational + data with no server-side anonymization +- a route, websocket request, local API, static fallback, or file-serving path + exposes data without the expected auth boundary +- state-changing websocket/local API behavior is available to read-only or + anonymous users without a documented reason +- operator/customer/integration-controlled strings can reach `innerHTML` or HTML + attributes without escaping +- an XSS would expose a session token, API key, config secret, localStorage value, + or pending control-plane operation +- localStorage retains sensitive topology/configuration data beyond the browser + session or logout without a clear need + +Do not count these as findings by themselves: + +- static JS/CSS/images served without auth when they contain no operator data, + credentials, or secrets +- client-side redaction that is documented as screenshot/demo display redaction, + unless the same mode is used as the privacy boundary for anonymous/public + access +- admin-only config views that already redact integration secrets before sending + them to the browser +- `innerHTML` used only for fixed icons, fixed Bootstrap markup, or escaped values + +The audit-file section must include: + +- Heading: `Node Manager privacy, auth, and XSS audit`. +- Date, scope, exact searches or files reviewed, and any excluded generated/vendor + output. +- Summary bullets separating confirmed findings, hardening observations, and + not-findings. +- One subsection per finding with repo-relative `path:line`, short description, + exposure/threat, and recommended actions. +- A short localStorage/sessionStorage/cookie note, even when no sensitive + localStorage token is found. +- No placeholders, no `...`, and no unresolved `` tokens. + +## Step 6: Audit Introduction, Executive Summary, Conclusion, and Grade + +Use this step after the requested audit sections have enough findings to +summarize. Keep the write-up brief enough for release planning while preserving +the concrete evidence in the detailed sections below. + +At the beginning of the audit file, add: + +- `Introduction`: the audit date, overall scope, what was static-only, and major + exclusions such as Linux/Ubuntu vulnerabilities, live firewall state, or + deployment-specific controls not visible in the checkout. +- `Executive summary`: bullet points that signal the actionable issues found + later in the document. Include the highest-risk findings, non-findings that + matter to release confidence, and the recommended fix priority. +- An overall letter grade from `A` to `F`. Use the grade as a concise release + posture signal, not as a substitute for findings. + +At the end of the audit file, add: + +- `Conclusion`: restate the overall grade and explain the security posture in a + balanced way. +- Call out what LibreQoS is doing right, such as dependency health, auth layers, + TLS/Caddy direction, redaction of secrets, verifier-conscious eBPF bounds + checks, or any other strengths actually found in the detailed sections. +- Call out what needs work, prioritizing concrete risk over volume of findings. +- Keep recommendations aligned with the detailed findings. Do not introduce new + unsupported claims in the conclusion. + +Suggested grading rubric: + +- `A`: No confirmed high-impact release issues, strong auth/session boundaries, + validated input handling, and only minor hardening items remain. +- `B`: Good foundations with a few bounded, actionable hardening issues. +- `C`: Acceptable foundations, but multiple actionable issues remain in + control-plane, WebUI, privacy, panic/DoS, or observability paths. +- `D`: Security-critical issues are likely reachable or broad auth/privacy + boundaries are missing. +- `F`: Known unauthenticated compromise, widespread secret exposure, or a + release-blocking vulnerability with no mitigation. + +The introduction, executive summary, and conclusion must not contain +placeholders, speculative certainty, or findings that are absent from the body of +the audit. + +## Validation + +- Re-read the audit section before returning; remove placeholders and vague conclusions. +- After changing any repo file, run the repo's anti-slop review path and fix non-zero slop before finishing. diff --git a/SECURITY_AUDIT_1_TASKS.md b/SECURITY_AUDIT_1_TASKS.md new file mode 100644 index 00000000..0478442f --- /dev/null +++ b/SECURITY_AUDIT_1_TASKS.md @@ -0,0 +1,303 @@ +# Security Audit 1 Tasks + +Date: 2026-05-15 +Branch goal: move the current security branch from a solid B posture to an A-grade release posture. + +2026-06-15 branch note: `chore/security_audit_2026_06_15` intentionally +excludes the eBPF/XDP/TC structural changes and related `lqos_sys` Rust-side +changes from the May audit branch. Tasks that require those surfaces remain +deferred here until they can be reintroduced with live load validation. + +Source notes reviewed: + +- `SECURITY_AUDIT_2026_05_15_update1.md` +- `SECURITY_AUDIT_2026_05_15_DEEPSEEK.md` + +## Scope Rules + +- Assume the managed Caddy/HTTPS setup path is used. Do not spend this branch re-solving operator deployment exposure that is already handled by the Caddy setup path. +- Keep this list to repo-owned improvements that can be fixed, tested, or documented in this branch. +- Treat eBPF code size and verifier loadability as hard constraints. Prefer bounded checks, conservative pass behavior, counters, and tests over large parser expansions. +- Do not include OS, kernel, distro package, firewall, or live-host configuration findings in this branch task list. + +## P0: A-Ready Blockers + +### 1. Finish the remaining packet-capture stored-XSS cleanup in Node Manager + +Status: Done. Packet-capture IP/address labels now use DOM text nodes via tested helpers; node_manager tests and build-contract validation passed on 2026-05-15. + +Why: + +DeepSeek's highest-risk finding is still visible in the source: the circuit packet-capture dropdown concatenates IP addresses into `innerHTML`. Even if the update note intended this class of issue to be fixed, this exact sink should be removed before calling the branch A-ready. + +Paths: + +- `src/rust/lqosd/src/node_manager/js_build/src/circuit.js` +- `src/rust/lqosd/src/node_manager/js_build/src/circuit_packet_capture_dom.mjs` +- `src/rust/lqosd/src/node_manager/js_build/src/circuit_packet_capture_dom.test.mjs` + +Work: + +- Replace the packet-capture dropdown construction with DOM text nodes, or add a shared escaping helper and apply it to the IP/address insertions. +- Review remaining `innerHTML` assignments that mix trusted markup with operator-controlled values. Keep static icon markup separate from untrusted text. +- Add a focused regression check for the packet-capture IP path using an IP-like string containing HTML metacharacters. + +Done when: + +- Packet-capture IP/address labels render through DOM text APIs, not `innerHTML`. +- Node Manager build-contract validation passes. + +### 2. Clamp and warn on all remaining 32-bit telemetry narrowing + +Status: Done. NetFlow 9 timestamps and LTS2/Insight client-side telemetry conversions now clamp with warnings while preserving current wire field widths; focused Rust tests and `cargo check -p lqosd` passed on 2026-05-15. + +Why: + +The update audit still calls out NetFlow 9 timestamps and LTS2 stats submission counters that narrow wider values without a documented clamp. This is not an unauthenticated exploit, but A-grade security should avoid silent wraparound in operational telemetry. + +Paths: + +- `src/rust/lqosd/src/throughput_tracker/flow_data/netflow9/protocol/header.rs` +- `src/rust/lqosd/src/throughput_tracker/stats_submission.rs` +- Insight/LTS2 shared protocol types are out of scope for this branch unless the other end is coordinated. + +Work: + +- Reuse or share the NetFlow 5 clamp helpers for NetFlow 9 `sys_uptime` and `unix_secs`. +- Add saturation helpers for LTS2 `u64 -> u32`, `u64 -> i32`, and RTT millisecond conversions. +- Preserve the current Insight/LTS2 wire field widths and clamp client side rather than widening protocol fields. +- Log low-rate warnings when saturation happens. +- Document any protocol limit that must remain 32-bit for compatibility. + +Done when: + +- Values above `u32::MAX`, `i32::MAX`, and non-finite RTT conversions are covered by tests and cannot silently wrap. +- Any required Insight/LTS2 compatibility decision is written down in the audit update. + +### 3. Add an auth surface regression gate for Node Manager + +Status: Done. Added the missing HTML route build-contract check, websocket same-origin guard, template-page block, and CORS regression test; existing cookie and read-only dashboard tests were rerun on 2026-05-15. + +Why: + +The branch fixed important auth behavior: no anonymous WebUI mode, CORS removed, session cookies hardened, durable session token no longer copied into websocket auth, and read-only dashboard writes denied. A-grade security needs a regression gate so these do not slip back. + +Paths: + +- `src/rust/lqosd/src/node_manager/auth.rs` +- `src/rust/lqosd/src/node_manager/run.rs` +- `src/rust/lqosd/src/node_manager/static_pages.rs` +- `src/rust/lqosd/src/node_manager/ws.rs` +- `src/rust/lqosd/src/node_manager/local_api/` + +Work: + +- Add tests or a build-contract check proving every shipped `.html` page is routed through the authenticated/template path, except explicit login/health/first-run pages. +- Add checks for session cookie flags: `HttpOnly`, `SameSite=Lax`, and conditional `Secure` when SSL/Caddy mode is active. +- Add checks that CORS remains absent or non-credentialed on Node Manager. +- Add read-only authorization regression tests for dashboard theme save/delete and any other state-changing websocket/local-api route touched in this branch. +- Check websocket upgrade `Origin` handling. If cross-origin cookie-authenticated upgrades are accepted, add an Origin allowlist or document and test why the current browser cookie behavior is sufficient. + +Done when: + +- Auth, cookie, CORS, route whitelist, and read-only write behavior are covered by repeatable tests or build-contract checks. + +### 4. Remove panic and silent-failure paths on operator-controlled inputs + +Status: Deferred on the 2026-06-15 branch. The original mixed commit touched +operator-input hardening and XDP/TC cleanup visibility in the same patch, so it +was left out of this branch to keep `lqos_sys` unchanged. + +Why: + +DeepSeek found several low-to-medium reliability/security issues where malformed operator files, imported API data, or cleanup failures panic or silently hide an operational failure. These are not direct remote compromise vectors, but they are exactly the sort of hardening work that separates a B from an A. + +Paths: + +- `src/rust/uisp_integration/src/ip_ranges.rs` +- `src/rust/uisp_integration/src/uisp_types/uisp_device.rs` +- `src/rust/lqos_queue_tracker/src/queue_structure/queue_node.rs` +- `src/rust/lqos_heimdall/src/watchlist.rs` +- `src/rust/lqos_sys/src/bpf_map.rs` +- `src/rust/lqos_sys/src/lqos_kernel.rs` +- `src/rust/lqos_bus/src/bus/unix_socket_server.rs` + +Work: + +- Replace config/API parse `unwrap()` calls with typed errors or warning-plus-skip behavior. +- Return or log parse errors for malformed `queueStructure.json` instead of silently creating an empty network. +- Log Heimdall watch insert/delete failures. +- Count or log BPF map delete failures during clear operations. +- Log XDP/TC detach fallback failures without changing live behavior. +- Log Unix socket cleanup failures before retry or bind failure. + +Done when: + +- Malformed operator-managed inputs produce actionable errors or warnings, not panics or silent empty state. +- Cleanup failures are visible in logs without causing unnecessary service disruption. + +## P1: Strong A-Grade Hardening + +### 5. Add low-cost eBPF backpressure visibility + +Status: Deferred on the 2026-06-15 branch because it requires eBPF map and +`lqos_sys` reader changes. + +Why: + +The RTT `flowbee_events` ring-buffer path still ignores `bpf_ringbuf_output` failure. Heimdall now checks copy/output errors, but drop visibility should be consistent. This should be done with extreme care because recent eBPF changes already hit verifier and size limits. + +Paths: + +- `src/rust/lqos_sys/src/bpf/common/flows.h` +- `src/rust/lqos_sys/src/bpf/common/heimdall.h` +- `src/rust/lqos_sys/src/bpf/common/throughput.h` +- userspace readers that can expose low-rate pressure counters + +Work: + +- Add the smallest practical per-CPU drop counter for RTT ring-buffer failures. +- Avoid per-packet logging. +- Reuse existing map-pressure or insert-pressure reporting patterns if possible. +- Verify the BPF object still loads after every change. + +Done when: + +- Flow RTT event loss is observable under ring-buffer pressure. +- The eBPF program still builds and loads. + +### 6. Expand malformed-traffic coverage without growing the hot path recklessly + +Status: Deferred on the 2026-06-15 branch because it changes XDP/TC packet +parsing and verifier-sensitive eBPF code. + +Why: + +The May audit branch fixed IPv4 header length, total length, version, and +fragment handling, but this 2026-06-15 branch does not carry that eBPF parser +work. The audit still points at IPv6 extension headers, IPv6 fragments, MPLS +parser coverage, and fragment policy. Full parser expansion may be too +expensive for XDP, so the task should be conservative. + +Paths: + +- `src/rust/lqos_sys/src/bpf/common/dissector.h` +- `src/rust/lqos_sys/src/bpf/common/dissector_tc.h` +- `src/rust/lqos_sys/src/bpf/lqos_kern.c` + +Work: + +- Add tests or verifier-focused fixtures for IPv6 packets with Hop-by-Hop, Routing, Destination Options, AH, and Fragment headers. +- Prefer "detect and pass without flow tracking" for IPv6 fragments and extension-header cases unless a small bounded parser fits the verifier and size budget. +- Add MPLS stacked-label tests for the current pass behavior. +- Document the fragment policy: what is shaped, what is passed, and why. + +Done when: + +- Malformed and extension-header traffic has explicit, tested behavior. +- No new eBPF size or verifier regression is introduced. + +### 7. Move sensitive browser operational drafts out of durable localStorage + +Status: Done. Network-mode drafts and pending-operation state now use tab-scoped `sessionStorage`, legacy durable values are migrated and removed, apply/confirm/revert/rollback/logout clear operational keys, harmless preferences remain durable, and node_manager storage/build-contract validation passed on 2026-05-15. + +Why: + +The update audit found no durable session tokens in browser storage, which is good. Remaining `localStorage` usage still includes operational drafts and pending network-mode state. On shared operator workstations, A-grade posture should reduce how much deployment detail survives logout or browser restart. + +Paths: + +- `src/rust/lqosd/src/node_manager/js_build/src/config_interface.js` +- dashboard preference storage under `src/rust/lqosd/src/node_manager/js_build/src/lq_js_common/dashboard/` +- template/login logout paths that can clear storage + +Work: + +- Classify each `localStorage` key as harmless preference, short-lived operational draft, or sensitive deployment detail. +- Move short-lived operational draft/pending-operation state to `sessionStorage` where practical. +- Clear sensitive draft keys on confirm, rollback, cancel, logout, and page unload where appropriate. +- Keep harmless user preferences such as theme and colorblind mode durable. + +Done when: + +- No interface, VLAN, pending operation, or deployment-change draft persists indefinitely in `localStorage`. +- Harmless preferences still work across reloads. + +### 8. Add a Content-Security-Policy appropriate for the current UI + +Status: Done. Authenticated templated pages and standalone login/first-run pages now emit an enforced CSP that keeps scripts self-hosted, preserves the existing inline template/style requirements, permits current websocket/API-docs/Insight tile needs, blocks framing, and is covered by Rust CSP tests plus `lqosd` check/clippy and node_manager build-contract validation on 2026-05-15. + +Why: + +CSP will not replace XSS fixes, but it reduces blast radius if a future trusted-HTML sink regresses. The current UI uses Bootstrap, FontAwesome, local scripts, and some inline bootstrap/theme snippets, so the policy should be practical rather than aspirational. + +Paths: + +- `src/rust/lqosd/src/node_manager/template.rs` +- `src/rust/lqosd/src/node_manager/static2/template.html` +- login and first-run pages if they bypass the main template + +Work: + +- Start with a report-only policy if needed. +- Prefer self-hosted scripts/styles and avoid enabling remote script sources. +- Keep any required inline allowances documented and minimized. +- Add a smoke test that the main UI pages still render. + +Done when: + +- A CSP header is present on authenticated UI pages without breaking the existing Node Manager. + +### 9. Keep Caddy/API loopback behavior guarded, but do not duplicate setup work + +Status: Done. Managed Caddy Caddyfile tests now assert loopback API/WebUI upstreams and runtime enable/disable tests continue to preserve `lqos_api` restarts; operator docs now identify `LQOS_API_LISTEN` as an advanced override that should stay unset on managed Caddy systems unless direct API exposure is intentional and separately protected. + +Why: + +The branch assumes Caddy is set up. The useful repo-owned hardening is to prevent accidental drift from the intended Caddy/loopback shape, not to treat Caddy setup itself as missing. + +Paths: + +- `src/rust/lqos_setup/src/ssl.rs` +- `src/rust/lqos_setup/src/web.rs` +- `src/rust/lqosd/src/node_manager/local_api/ssl.rs` +- sibling `lqos_api` listener code if coordinated in this branch + +Work: + +- Keep tests proving Caddy mode writes a loopback API listener and restarts the API service when the mode changes. +- If `LQOS_API_LISTEN` remains an advanced override, make the override noisy when it defeats Caddy loopback isolation. +- Document that the override is for advanced deployments and should not be set on managed Caddy systems. +- Do not make the first-run setup flow unusable for headless installs unless there is an explicit product decision. + +Done when: + +- Managed Caddy mode has tests and docs that preserve loopback API isolation. +- Any override that bypasses it is intentional and visible. + +## P2: Audit Evidence and Release Gate + +### 10. Close the audit with repeatable evidence + +Why: + +The final grade should be based on proof, not just code inspection. + +Work: + +- Run the focused Rust checks for touched crates. +- Run Node Manager build-contract validation after frontend changes. +- Run `cargo audit`, `cargo machete`, and a dependency tree check after dependency or protocol changes. +- For eBPF changes, verify build and load behavior in the same way used to confirm the current eBPF fix. +- Update the audit document with exact commands, outcomes, and remaining accepted risks. + +Done when: + +- The branch has an evidence-backed security summary that explains why the remaining risks are accepted for the release target. + +## Deferred Unless Time Allows + +- Do not retire `serde_cbor` in this security branch; the Insight/LTS2 protocol counterpart makes that too risky without coordinated work. +- Replace other unmaintained crates (`fxhash`, `bincode`, transitive `paste`) only when the migration is small or already planned. Track them as maintenance if they do not carry an active advisory. +- Add optional server-side redaction for read-only staff accounts as a product privacy feature. This is useful, but not required to preserve the intended trusted-support-staff model. +- Align userspace flow/host cache capacities with BPF LRU behavior if it can be done without destabilizing flow reporting. Treat this as observability correctness, not an A-blocking security issue. diff --git a/SECURITY_AUDIT_2026_05_15.md b/SECURITY_AUDIT_2026_05_15.md new file mode 100644 index 00000000..09007a69 --- /dev/null +++ b/SECURITY_AUDIT_2026_05_15.md @@ -0,0 +1,1083 @@ +## Introduction + +This audit is a basic pre-release security review of LibreQoS as of +2026-05-15. It focuses on dependency advisories, Rust dependency hygiene, +control-plane exposure, bridged-interface and eBPF malformed-traffic handling, +panic/error/type-loss paths, and Node Manager privacy, authentication, and XSS +risk. + +The review is static unless a section explicitly says otherwise. It does not +cover Ubuntu/Linux distribution vulnerabilities, live firewall state, NIC driver +behavior, or private deployment-specific controls that are not visible in this +checkout. + +## Executive summary + +- Overall grade: **C**. LibreQoS has several strong security foundations, but + the current release state includes multiple actionable control-plane and WebUI + issues that should be fixed or explicitly risk-accepted before release. +- No CVE-class Rust dependency vulnerability was confirmed as reachable during + the dependency audit. The `rand` soundness advisory has been remediated by + updating the locked 0.8 dependency line to `rand 0.8.6`, and one likely unused + dependency should be removed. +- The Caddy/TLS path is generally well-directed for the WebUI, but the sibling + API service can still bind directly on all interfaces, malformed bearer headers + can panic API authentication, WebUI/local API CORS is too broad, and WebUI + login lacks rate limiting. +- The eBPF packet parsers show good verifier-conscious bounds checking for + common encapsulation paths, but malformed traffic can still create operational + DoS or visibility loss through packet-rate debug logging, IPv4 fragment/header + edge cases, and non-LRU map exhaustion. +- The panic/type-loss review found request-time panic paths and unchecked numeric + narrowing that can lose operational data in queue stats and NetFlow export. +- Node Manager has the highest concentration of release-relevant WebUI risk: + anonymous/read-only paths can retrieve raw subscriber and topology data, + operator-controlled strings reach `innerHTML`, the session cookie is readable + by JavaScript, and read-only users can write dashboard themes that can become + stored XSS. +- Recommended release priority is to first fix WebUI XSS/session-cookie exposure, + server-side anonymization for anonymous/demo mode, API direct-listener/TLS + bypass risk, malformed auth-header panic, and production packet-path debug + logging. + +## Rust dependency audit + +Date: 2026-05-15 + +Scope: Rust workspace under `src/rust/`, using `src/rust/Cargo.toml` and +`src/rust/Cargo.lock`. + +Commands run: + +- `cargo audit` +- `cargo machete` +- `cargo tree --locked --workspace --depth 1` +- `cargo update -p rand@0.8.5 --precise 0.8.6` +- `cargo tree --locked -p lqosd -e features` +- `cargo check -p lqos_probe -p lqosd` + +### Summary + +- `cargo audit` scanned 549 locked crate dependencies. +- No CVE-class vulnerability was reported. +- The previous `rand` soundness advisory no longer appears in `cargo audit` + after updating `rand 0.8.5` to `rand 0.8.6`. +- Direct `rand` imports now disable default features and re-enable only `std` + and `std_rng`, which are required for the current `thread_rng` and `random` + calls. The resolved graph still enables `rand/default` through upstream + `tungstenite` and `cookie` dependencies, so feature minimization is limited + to LibreQoS' direct imports unless those upstream crates are patched or + upgraded. +- `cargo machete` found one likely unused dependency: `tokio` in + `lqos_network_devices`. +- The remaining `cargo audit` warnings were maintenance-only notices and are + not counted as security issues here. + +### Dependency findings and triage notes + +#### Resolved: RUSTSEC-2026-0097 / GHSA-cq8v-f236-94qc `rand` soundness advisory + +Paths importing or using the affected dependency/API: + +- `src/rust/lqosd/Cargo.toml` imports + `rand = { version = "0.8.6", default-features = false, features = ["std", "std_rng"] }`. +- `src/rust/lqosd/src/node_manager/auth.rs` uses + `rand::thread_rng().fill_bytes(...)` for session-key generation. +- `src/rust/lqosd/src/node_manager/ws/single_user_channels.rs` uses + `rand::random::()` for chatbot request IDs. +- `src/rust/lqos_probe/Cargo.toml` imports + `rand = { version = "0.8.6", default-features = false, features = ["std", "std_rng"] }`. +- `src/rust/lqos_probe/src/lib.rs` uses `rand::random` for ICMP ping IDs. +- `src/rust/lqosd/Cargo.toml` also imports `tungstenite`, + `tokio-tungstenite`, and `axum-extra`, which appear in the `cargo audit` + dependency tree above `rand 0.8.6`. + +Short description: + +`rand` versions including `0.8.5` have a soundness advisory involving +`rand::thread_rng` / `rand::rng`. The trigger requires the `log` and +`thread_rng` features, a custom logger, that logger calling the RNG, and the RNG +reseeding while called from the logger. LibreQoS has affected API calls, but the +audit did not find a custom `log::Log` implementation or `log::set_logger` path +in `src/rust/`, so the full trigger path was not found. + +Remediation: + +- Updated the locked 0.8 line with + `cargo update -p rand@0.8.5 --precise 0.8.6`. +- Minimized LibreQoS' direct `rand` feature requests to `std` and `std_rng`. +- Re-ran `cargo audit`; the `rand` advisory is no longer reported. +- Re-ran `cargo check -p lqos_probe -p lqosd`; both crates compile with the + updated dependency. + +#### Unused dependency: `tokio` in `lqos_network_devices` + +Path: + +- `src/rust/lqos_network_devices/Cargo.toml` imports `tokio = { workspace = true }`. + +Short description: + +`cargo machete` reports `tokio` as unused in `lqos_network_devices`. A source +search found no `tokio` references under `src/rust/lqos_network_devices/`. +This is not a security issue, but it is avoidable dependency surface. + +Recommended actions: + +- Remove the `tokio` dependency from `src/rust/lqos_network_devices/Cargo.toml`. +- Run `cargo check -p lqos_network_devices`. +- Re-run `cargo machete` to confirm the dependency list is clean. + +### Non-security audit warnings + +The following `cargo audit` results are maintenance-only warnings, not security +findings under this audit policy: + +- `bincode 1.3.3` is unmaintained; imported by `lqosd`. +- `fxhash 0.2.1` is unmaintained; imported by `lqosd` and + `lqos_network_devices`. +- `paste 1.0.15` is unmaintained; transitive through `default-net` / + `netlink-packet-*`. +- `serde_cbor 0.11.2` is unmaintained; imported by several LibreQoS crates and + `lqos_bus`. + +These should be tracked as modernization work, not listed as release security +findings unless a separate vulnerability is reported for them. + +## Network control-plane audit + +Date: 2026-05-15 + +Scope assumptions: + +- The Caddy / SSL / TLS option is installed. +- Linux, Ubuntu, kernel, and distribution package vulnerabilities are out of + scope for this section. +- The control interface is in scope. The two bridge interfaces, whether XDP or + Linux bridge backed by eBPF, are out of scope for this section. +- An open listener is not a vulnerability by itself. This section looks for + exploitability, unauthorized access, credential exposure, authorization + bypass, or remotely triggerable failure on the control plane. +- The sibling `../../lqos_api/` repo is included because it is exposed behind + the managed Caddy configuration. + +Files and directories reviewed: + +- `docs/v2.0/https-caddy.md` +- `docs/v2.0/api.md` +- `src/rust/lqos_setup/src/ssl.rs` +- `src/rust/lqos_setup/src/web.rs` +- `src/rust/lqosd/src/node_manager/run.rs` +- `src/rust/lqosd/src/node_manager/auth.rs` +- `src/rust/lqosd/src/node_manager/local_api.rs` +- `src/rust/lqosd/src/node_manager/static_pages.rs` +- `src/rust/lqos_config/src/authentication.rs` +- `../../lqos_api/src/main.rs` +- `../../lqos_api/src/web.rs` +- `../../lqos_api/src/web_security.rs` +- `../../lqos_api/README.md` + +### Summary + +- The managed Caddy configuration disables the Caddy admin API, proxies the + WebUI to `127.0.0.1:9123`, and proxies `/api/v1/*` to `127.0.0.1:9122`. +- The WebUI runtime listener is configured to move to loopback for the Caddy + path. That loopback listener is not a finding. +- The sibling `lqos_api` service still binds directly to `:::9122`. If that + port is reachable on the control interface, authenticated API traffic can + bypass the Caddy/TLS path. Runtime reachability was not verified in this + code audit. +- Three control-plane findings and one reachability-unknown exposure are listed + below. Public API documentation and the explicit anonymous read-only demo mode + are recorded as observations, not findings by themselves. + +### Findings + +#### Reachability unknown: direct `lqos_api` listener can bypass the Caddy/TLS path + +Path: + +- `../../lqos_api/src/main.rs` + +Short description: + +`lqos_api` binds its HTTP server to `:::9122`. The managed Caddy configuration +proxies API traffic to `127.0.0.1:9122`, but the API process itself also remains +able to listen on all interfaces unless deployment firewalling blocks it. + +Exposure / threat: + +The API binds all interfaces while Caddy proxies the intended HTTPS path to +localhost. If port `9122` is reachable on the control interface, a client can +send the `x-bearer` credential over direct HTTP instead of the Caddy-protected +HTTPS path. This audit verified the code-level listener and Caddy upstream, but +did not verify runtime firewall or socket exposure on an installed host. + +Recommended actions: + +- Make the `lqos_api` listen address configurable and default it to + `127.0.0.1:9122` when the Caddy option is installed. +- Update the Caddy/setup integration and API documentation so remote operators + use only the HTTPS `/api/v1/` path. +- Add install-time firewall guidance or service hardening that blocks direct + control-interface access to `9122` when Caddy is enabled. + +#### Malformed `x-bearer` header can panic API authentication + +Path: + +- `../../lqos_api/src/web_security.rs` + +Short description: + +The API authentication middleware calls `header.to_str().unwrap()` while +processing the unauthenticated `x-bearer` header. + +Exposure / threat: + +A remote client can send a malformed header value that fails UTF-8 conversion. +Authentication should reject that request, but the current code can panic while +handling unauthenticated input. Even if Axum/Tokio limits the blast radius to a +request task or connection, this is a remotely triggerable control-plane failure +path. + +Recommended actions: + +- Replace the `unwrap()` with explicit error handling that returns + `401 Unauthorized` for invalid or missing bearer headers. +- Keep malformed authentication input on the same path as other auth failures: + no panic, no stack trace, and no different response body that helps probing. +- After fixing, add a small test for a non-UTF-8 or otherwise invalid + `x-bearer` value. + +#### WebUI and local API use credentialed permissive CORS + +Paths: + +- `src/rust/lqosd/src/node_manager/run.rs` +- `src/rust/lqosd/src/node_manager/static_pages.rs` +- `src/rust/lqosd/src/node_manager/local_api.rs` +- `src/rust/lqosd/src/node_manager/auth.rs` + +Short description: + +The WebUI and local API install `CorsLayer::very_permissive()`, a permissive +credentialed CORS policy. The WebUI session uses a `User-Token` cookie with +`SameSite=Lax`, but without `Secure` or `HttpOnly`. + +Exposure / threat: + +I did not find a documented cross-origin WebUI client in the reviewed code. For +a cookie-authenticated control-plane UI, permissive credentialed CORS grants +browser read access to origins outside the WebUI's own origin whenever the +browser sends the `User-Token` cookie. `SameSite=Lax` limits common +unrelated-site subresource requests, but this policy is still broader than the +reviewed WebUI needs. The local API and WebUI should not grant credentialed +CORS to arbitrary origins without a documented client need. + +Recommended actions: + +- Remove CORS from same-origin WebUI/local API routes unless a concrete + supported cross-origin client requires it. +- If cross-origin access is required, restrict allowed origins to configured + operator hosts and avoid credentialed wildcard or origin-mirroring behavior. +- Add origin or CSRF checks for state-changing browser routes. +- Set session cookies with `Secure` when served behind HTTPS and `HttpOnly` + unless browser JavaScript truly needs to read the cookie. + +#### WebUI login lacks rate limiting + +Paths: + +- `src/rust/lqosd/src/node_manager/auth.rs` +- `src/rust/lqos_config/src/authentication.rs` + +Short description: + +The WebUI login path checks passwords with Argon2id for current hashes and +upgrades older SHA-256 hashes after successful login. The reviewed code did not +show per-IP, per-account, or global throttling for repeated failed login +attempts to the public `/doLogin` route. + +Exposure / threat: + +Under the Caddy setup, the login form is reachable through the control-plane +HTTPS entrypoint. An unauthenticated client can repeatedly submit passwords to +`/doLogin`; the server-side password hash is strong, but the reviewed code does +not throttle repeated failures before each verification attempt. + +Recommended actions: + +- Add rate limiting or exponential backoff for failed `/doLogin` attempts, + keyed by source address and username. +- Log repeated failures in a way operators can act on without logging submitted + passwords. + +### Observations / not findings + +- `src/rust/lqos_setup/src/ssl.rs` renders a managed Caddyfile with + `admin off`, WebUI upstream `127.0.0.1:9123`, and API upstream + `127.0.0.1:9122`. +- `docs/v2.0/https-caddy.md` documents moving the WebUI runtime listener to + `127.0.0.1:9123` when HTTPS is enabled. +- `../../lqos_api/src/web.rs` merges Swagger UI at `/api-docs`. This exposes + endpoint shape, not credentials or control actions, and is not counted as a + vulnerability by itself. +- `allow_anonymous` is an explicit read-only public/demo mode in the WebUI + authentication configuration. It is not counted as a finding when the operator + intentionally enables that mode. +- `src/rust/lqos_setup/src/web.rs` binds the setup web service to + `0.0.0.0:9123` and uses a setup token. Because this section assumes the + runtime Caddy option is already installed, first-run setup exposure is left + for a later setup/lifecycle audit unless a concrete bypass is found. + +## Bridged interface / eBPF malformed-traffic audit + +Date: 2026-05-15 + +Scope assumptions: + +- The two bridged interfaces are in scope for this section, including XDP, + TC/eBPF, pinned maps, ring-buffer events, and userspace consumers of eBPF + output. +- The control interface and Caddy/API/WebUI exposure are covered by the prior + section and are out of scope here. +- Linux, Ubuntu, kernel, and NIC driver vulnerabilities are out of scope. This + section reviews LibreQoS packet parser behavior, map pressure, debug logging, + and userspace handling of eBPF output. +- No live XDP/TC attach-detach, pinned-map cleanup, or packet fuzzing was + performed. Findings are based on static review of in-repo code. + +Files and directories reviewed: + +- `src/rust/lqos_sys/src/bpf/common/debug.h` +- `src/rust/lqos_sys/src/bpf/common/dissector.h` +- `src/rust/lqos_sys/src/bpf/common/dissector_tc.h` +- `src/rust/lqos_sys/src/bpf/common/flows.h` +- `src/rust/lqos_sys/src/bpf/common/heimdall.h` +- `src/rust/lqos_sys/src/bpf/common/lpm.h` +- `src/rust/lqos_sys/src/bpf/common/throughput.h` +- `src/rust/lqos_sys/src/bpf/common/maximums.h` +- `src/rust/lqos_sys/src/lqos_kernel.rs` +- `src/rust/lqosd/src/throughput_tracker/flow_data/flow_analysis/kernel_ringbuffer.rs` +- `src/rust/lqosd/src/throughput_tracker/tracking_data.rs` +- `src/rust/lqos_heimdall/src/perf_interface.rs` +- `src/rust/lqos_heimdall/src/timeline.rs` +- `src/rust/lqos_heimdall/src/pcap.rs` + +Review searches: + +- `rg "bpf_debug\\(|frag_off|ihl|tot_len|doff|BPF_MAP_TYPE_HASH|BPF_MAP_TYPE_PERCPU_HASH|BPF_MAP_TYPE_LRU|MAX_FLOWS|MAX_TRACKED_IPS|bpf_ringbuf_output|bpf_probe_read_kernel" src/rust/lqos_sys/src src/rust/lqosd/src/throughput_tracker src/rust/lqos_heimdall/src` + +### Summary + +- The XDP/TC packet dissectors use explicit `data_end` bounds checks and bounded + VLAN/MPLS loops. The review did not find memory-unsafe packet reads in the + normal parser path. +- Parser failures in `xdp_prog` return `XDP_PASS`; parser failures in + `tc_iphash_to_cpu` return `TC_ACT_OK`. Unknown non-IP traffic therefore + passes unshaped in the reviewed XDP/TC paths. +- Five malformed-traffic / resource-exhaustion findings are listed below. + Packet-rate `bpf_trace_printk` is reachable through the `bpf_debug` macro + from malformed short UDP/ICMP paths. +- Several findings affect observability and flow analysis more than packet + forwarding. They can still break LibreQoS operationally by hiding current + flow/host state, filling pinned maps, or burning CPU on bridged-interface + traffic. + +### Findings + +#### Malformed packets can trigger packet-rate BPF trace logging + +Paths: + +- `src/rust/lqos_sys/src/bpf/common/debug.h` +- `src/rust/lqos_sys/src/bpf/common/dissector.h` +- `src/rust/lqos_sys/src/bpf/common/flows.h` +- `src/rust/lqos_sys/src/bpf/common/throughput.h` + +Short description: + +`bpf_debug(...)` expands directly to `bpf_trace_printk(...)`. Some call sites +are behind `VERBOSE` or `TRACING`, but several error paths reachable from packet +handling are not. Examples include truncated UDP/ICMP headers in +`dissector.h` and map insertion failures in `flows.h` and `throughput.h`. + +Exposure / threat: + +An attacker on a bridged interface can send malformed or high-cardinality +traffic that repeatedly hits these error paths. `bpf_trace_printk` is expensive +and writes into the kernel tracing path; at packet rate this can consume CPU and +trace-buffer bandwidth on the shaping host. Once map-pressure findings below +are triggered, failed insertions can also create a second packet-rate logging +path. + +Recommended actions: + +- Compile `bpf_debug` to a no-op unless `VERBOSE` or `TRACING` is explicitly + enabled. +- Replace packet-rate error logging with counters in a bounded BPF map that + userspace can poll at a low rate. +- Treat any remaining `bpf_trace_printk` call in XDP/TC packet paths as a debug + build feature, not production behavior. + +#### IPv4 fragments and invalid IPv4 header lengths can pollute flow tracking + +Path: + +- `src/rust/lqos_sys/src/bpf/common/dissector.h` + +Short description: + +The XDP dissector verifies that an IPv4 header-sized region is present, then +uses `iph->ihl * 4` to locate TCP, UDP, or ICMP headers. The reviewed code does +not reject `ihl < 5`, does not verify the IPv4 total length against the captured +packet bounds, and does not skip L4 snooping for fragmented IPv4 packets. + +Exposure / threat: + +Malformed IPv4 packets can make the dissector derive L4 ports and TCP flags +from bytes that are not a valid L4 header. When those bytes make the apparent +TCP data offset large enough, the TCP timestamp parser can also run against +fragment payload rather than a real TCP options area. On the TCP path, this can +seed or update Flowbee records, retransmit counters, and RTT sampling inputs +with attacker-chosen fragment payload bytes. + +UDP and ICMP fragments can also be interpreted as flow traffic if enough +payload bytes are present. Those paths can create or update UDP/ICMP Flowbee +entries from fragment payload instead of a valid UDP or ICMP header. + +Recommended actions: + +- Validate IPv4 `version == 4`, `ihl >= 5`, and `l3offset + ihl * 4 <= data_end` + before any L4 header lookup. +- Validate IPv4 total length enough to ensure the parsed L4 header is inside the + IPv4 packet, not just inside the received frame. +- Skip L4 snooping and Flowbee updates for IPv4 fragments with non-zero fragment + offset or `MF` set. Continue IP-level LPM shaping if desired. +- Add a small packet corpus for malformed IPv4 IHL values, truncated TCP/UDP, + and fragmented IPv4 packets. + +#### UDP/ICMP spray can fill the non-LRU Flowbee map + +Paths: + +- `src/rust/lqos_sys/src/bpf/common/flows.h` +- `src/rust/lqos_sys/src/bpf/common/maximums.h` +- `src/rust/lqosd/src/throughput_tracker/tracking_data.rs` + +Short description: + +`flowbee` is a pinned `BPF_MAP_TYPE_HASH` with `MAX_FLOWS` entries. The UDP and +ICMP handlers create a new Flowbee entry whenever no entry exists, even when the +IP mapping result has `tc_handle == 0`. TCP has a guard for non-SYN packets with +no mapping, but UDP and ICMP do not have the same shaped-traffic guard. + +Exposure / threat: + +Traffic with many spoofed IPs and ports can fill `flowbee` with unshaped UDP or +ICMP entries. When the map is full, later legitimate flow insertions fail and +LibreQoS loses current flow, RTT, retransmit, and QoE visibility for real +traffic. Each failed insert also reaches a `bpf_debug` path, which can amplify +the logging DoS above. + +Recommended actions: + +- Do not create UDP/ICMP Flowbee entries when `tc_handle == 0`, unless a + documented feature requires unshaped flow visibility. +- Consider changing `flowbee` to an LRU map, or add a bounded admission policy + for unshaped UDP/ICMP. +- Expose Flowbee map pressure and insertion failures to userspace as counters, + not trace logs. +- Add tests or a packet-replay harness that confirms unshaped UDP/ICMP sprays do + not evict or block shaped TCP flow visibility. + +#### Spoofed IP spray can fill the per-host traffic map + +Paths: + +- `src/rust/lqos_sys/src/bpf/common/throughput.h` +- `src/rust/lqos_sys/src/bpf/common/maximums.h` + +Short description: + +`map_traffic` is declared as `BPF_MAP_TYPE_PERCPU_HASH` with +`MAX_TRACKED_IPS = 128000`. The comment says the map is LRU, but the declared +type is not an LRU map. `track_traffic` inserts a host counter for every parsed +IP host key, including unshaped traffic with `tc_handle == 0`. + +Exposure / threat: + +An attacker can send traffic with many spoofed source or destination IPs through +the bridge and fill the host counter map. Once full, new legitimate host +counters fail to insert, unknown-IP and per-host throughput visibility becomes +misleading, and every failed insertion can hit `bpf_debug("Failed to insert +flow")`. + +Recommended actions: + +- Change the map type to an LRU variant if eviction of old host counters is the + intended behavior, or correct the comment and add explicit map-pressure + handling. +- Avoid inserting unshaped hosts into `map_traffic` unless operator-facing + unknown-IP visibility requires it. +- Add userspace counters for insert failures and map occupancy so operators can + distinguish real quiet periods from map exhaustion. + +#### Heimdall packet capture copies a fixed 128 bytes and ignores event-backpressure errors + +Paths: + +- `src/rust/lqos_sys/src/bpf/common/heimdall.h` +- `src/rust/lqos_heimdall/src/perf_interface.rs` +- `src/rust/lqos_heimdall/src/pcap.rs` + +Short description: + +When Heimdall analysis mode is enabled for a watched IP, the eBPF path copies +`PACKET_OCTET_SIZE` bytes from the packet start into each event and sends the +event through `heimdall_events`. The copy length is fixed at 128 bytes, the +return from `bpf_probe_read_kernel` is ignored, and the return from +`bpf_ringbuf_output` is ignored. + +Exposure / threat: + +This path is conditional on Heimdall watch mode, so it is not a default +bridging exposure. When enabled, short or malformed watched packets can produce +zero-padded or incomplete packet dumps without any signal to userspace. High +rate watched traffic can also fill the ring buffer. The current eBPF path +ignores the `bpf_ringbuf_output` return value, and the reviewed userspace path +has a collected-event counter and missed-tick warning but no surfaced +ring-buffer drop counter for Heimdall captures. + +Recommended actions: + +- Clamp the packet-copy length to the available packet length and + `PACKET_OCTET_SIZE`. +- Check the return values from `bpf_probe_read_kernel` and `bpf_ringbuf_output` + and increment bounded counters for copy failures and dropped events. +- Surface Heimdall copy-failure and ring-buffer drop counters through the + existing Heimdall/lqosd status path when packet capture is active. + +### Observations / not findings + +- VLAN, PPPoE, and MPLS parsing uses bounded loops and `data_end` checks before + walking encapsulation headers. This limits parser runtime on stacked headers. +- Unknown non-IP traffic passes unshaped by design, preserving ARP, STP, IS-IS, + and similar bridge traffic. +- IPv6 extension headers and IPv6 fragments are not deeply parsed for Flowbee + L4 metrics. The reviewed path still performs IP-level LPM mapping for + unsupported protocols, so this is an observability gap rather than a shaping + bypass in the reviewed code. +- Flowbee RTT ring-buffer userspace handling validates event size before copying + and uses a bounded in-process queue with coalesced wakeups. +- Runtime packet fuzzing, pinned-map occupancy checks, and live bridge-interface + reachability were not performed in this stage. + +## Panic, error-handling, and type-loss audit + +Date: 2026-05-15 + +Scope: + +- Runtime Rust under `src/rust/`, current Python entrypoints under `src/`, and + sibling `../../lqos_api/src/` because the API is deployed behind Caddy. +- Tests, generated output, vendored bindings, and historical Python copies were + excluded unless a runtime path referenced them. +- This was a static source review. No live service, XDP/TC attach-detach, or + packet replay was performed. + +Review searches: + +- `rg -n "\bpanic!\(|\.unwrap\(|\.expect\(|unreachable!\(|todo!\(|unimplemented!\(|assert!\(|from_raw_parts|transmute|unsafe \{|as (u8|u16|u32|usize|i8|i16|i32|f32)|unwrap_or_default\(|except Exception|except:|pass$" src/rust src --glob '*.py' ../../lqos_api/src` +- `rg -n "as u32|as u16|as f32|partial_cmp\(.*\)\.unwrap|to_str\(\)\.unwrap|parse\(\)\.unwrap|try_into\(\)\.unwrap" src/rust ../../lqos_api/src` +- `rg -n "except Exception|except:|pass$|sys.exit|int\(|float\(" src --glob '*.py' --glob '!LibreQoS-old.py' --glob '!LibreQoS-ancient.py' --glob '!LibreQoS.py.new'` + +### Summary + +- Four confirmed findings are listed below: one authenticated request-time panic, + one request/websocket error-handling panic pattern, one queue-stat type-loss + issue, and one NetFlow export type-loss issue. +- Two reachability-unknown hardening items are listed separately: API bandwidth + float narrowing and RTT percentile sorting on floats. +- The malformed `x-bearer` panic at `../../lqos_api/src/web_security.rs:32` was + already recorded in the network control-plane section and is not duplicated as + a new finding here. +- I did not find a confirmed memory-unsafe unsoundness issue in the reviewed + runtime paths. The high-volume unsafe hits were mostly FFI wrappers, generated + libbpf bindings, or callbacks that check event size before `from_raw_parts`. + +### Findings + +#### Authenticated packet-capture download can panic on missing capture file + +Paths: + +- `src/rust/lqosd/src/node_manager/local_api.rs:44` +- `src/rust/lqosd/src/node_manager/local_api/packet_analysis.rs:29` +- `src/rust/lqosd/src/node_manager/local_api/packet_analysis.rs:35` + +Short description: + +The authenticated `/local-api/pcapDump/:id` route calls +`n_second_pcap(id).expect(...)` and later `ServeFile::try_call(...).expect(...)`. +An invalid, expired, or removed packet-capture session can panic the request task +instead of returning a normal HTTP error. + +Exposure / threat: + +This route is behind the WebUI auth layer, so it is not an unauthenticated remote +panic. A logged-in user, stale browser request, or automation using an old capture +ID can still trigger request-time failure on the control plane. If the panic +poisons shared state in the surrounding server path, the blast radius could be +larger than one failed download. + +Recommended actions: + +- Return `404 Not Found` or `410 Gone` when `n_second_pcap(id)` cannot resolve a + capture file. +- Convert `ServeFile::try_call` errors into a bounded `5xx` response and log the + underlying path/error once. +- Add a focused route test for a missing capture ID and for a removed capture + file. + +#### Flow-explorer websocket handlers panic when time sources fail + +Paths: + +- `src/rust/lqosd/src/node_manager/local_api/flow_explorer.rs:42` +- `src/rust/lqosd/src/node_manager/local_api/flow_explorer.rs:45` +- `src/rust/lqosd/src/node_manager/local_api/flow_explorer.rs:104` +- `src/rust/lqosd/src/node_manager/local_api/flow_explorer.rs:107` +- `src/rust/lqosd/src/node_manager/local_api/flow_explorer.rs:117` +- `src/rust/lqosd/src/node_manager/local_api/flow_explorer.rs:120` +- `src/rust/lqosd/src/node_manager/ws.rs:665` +- `src/rust/lqosd/src/node_manager/ws.rs:674` +- `src/rust/lqosd/src/node_manager/ws.rs:683` + +Short description: + +The flow-explorer timeline builders use `expect(...)` on `time_since_boot()` and +`unix_now()`. These functions are called from websocket message handlers for ASN, +country, and protocol timelines. + +Exposure / threat: + +The direct trigger is an operating-system time retrieval failure, not attacker +controlled input. Still, once the condition exists, any authenticated websocket +request for these timeline views can panic instead of returning an empty/error +payload. This is incorrect request-time error handling on a control-plane feature. + +Recommended actions: + +- Return `Result, Error>` from the timeline builders and send a + structured websocket error when the clock calls fail. +- Reuse a small helper that computes boot time once with explicit logging. +- Add a focused unit test for the transport conversion path and a websocket test + for the error response if the time helper is injectable. + +#### Queue tracker silently narrows kernel qdisc counters + +Paths: + +- `src/rust/lqos_queue_tracker/src/queue_types/tc_cake.rs:102` +- `src/rust/lqos_queue_tracker/src/queue_types/tc_cake.rs:117` +- `src/rust/lqos_queue_tracker/src/queue_types/tc_cake.rs:187` +- `src/rust/lqos_queue_tracker/src/queue_types/tc_cake.rs:198` +- `src/rust/lqos_queue_tracker/src/queue_types/tc_cake.rs:206` +- `src/rust/lqos_queue_tracker/src/queue_types/tc_fq_codel.rs:61` +- `src/rust/lqos_queue_tracker/src/queue_types/tc_fq_codel.rs:62` +- `src/rust/lqos_queue_tracker/src/queue_types/tc_htb.rs:49` +- `src/rust/lqos_queue_tracker/src/queue_types/tc_htb.rs:50` +- `src/rust/lqos_queue_tracker/src/queue_types/tc_mq.rs:37` +- `src/rust/lqos_queue_tracker/src/queue_types/tc_mq.rs:38` + +Short description: + +The qdisc JSON parsers read `tc -s -j` values as `u64` and then cast many packet, +drop, backlog, flow, and memory counters to `u32` or `u16` with `as`. Rust's +integer narrowing casts wrap modulo the destination type, so large counters can +silently become smaller values. + +Exposure / threat: + +Busy shapers can exceed 32-bit packet/drop counters during normal operation. A +traffic flood can make this happen faster. Wrapped queue stats can hide drops, +mislead capacity and QoE views, and produce incorrect data for downstream +operator or Insight decisions. This is data loss rather than memory corruption. + +Recommended actions: + +- Keep kernel counters as `u64` through the queue tracker, bus messages, API + serialization, and UI consumers unless the kernel field is truly bounded. +- Where a protocol/UI contract must stay narrower, use `try_from` with explicit + clamp-and-warn behavior instead of unchecked `as` casts. +- Add qdisc parser fixtures with values above `u32::MAX` and `u16::MAX`. + +#### NetFlow 5 export can wrap flow counts, byte counts, and timestamps + +Paths: + +- `src/rust/lqosd/src/throughput_tracker/flow_data/netflow5/mod.rs:69` +- `src/rust/lqosd/src/throughput_tracker/flow_data/netflow5/protocol.rs:83` +- `src/rust/lqosd/src/throughput_tracker/flow_data/netflow5/protocol.rs:84` +- `src/rust/lqosd/src/throughput_tracker/flow_data/netflow5/protocol.rs:85` +- `src/rust/lqosd/src/throughput_tracker/flow_data/netflow5/protocol.rs:86` +- `src/rust/lqosd/src/throughput_tracker/flow_data/netflow5/protocol.rs:96` +- `src/rust/lqosd/src/throughput_tracker/flow_data/netflow5/protocol.rs:97` +- `src/rust/lqosd/src/throughput_tracker/flow_data/netflow5/protocol.rs:119` +- `src/rust/lqosd/src/throughput_tracker/flow_data/netflow5/protocol.rs:120` + +Short description: + +The NetFlow 5 exporter narrows accumulator length, packet counters, byte counters, +and nanosecond timestamps to `u16` or `u32` with unchecked casts. One direction +converts timestamps to milliseconds before narrowing; the reverse record narrows +nanoseconds directly. + +Exposure / threat: + +NetFlow export is optional, but when enabled it can silently emit wrapped or +inconsistent accounting for long-lived or high-throughput flows. External +collectors may then undercount traffic, mis-order flow times, or make billing and +abuse-analysis decisions from corrupted export records. + +Recommended actions: + +- Split NetFlow batches before the record count exceeds the protocol limit and + avoid unchecked `usize -> u16` casts. +- Convert timestamps consistently to the expected NetFlow units before narrowing. +- For NetFlow 5's inherently 32-bit fields, clamp with a warning or emit delta + records before counters exceed protocol capacity. +- Prefer NetFlow 9/IPFIX-style export for counters that need wider fields. + +### Reachability unknown / hardening items + +#### API site-speed changes narrow unbounded `f64` request values to `f32` + +Paths: + +- `../../lqos_api/src/api/network_json.rs:126` +- `../../lqos_api/src/api/network_json.rs:127` +- `../../lqos_api/src/api/network_json.rs:128` +- `../../lqos_api/src/api/network_json.rs:129` +- `../../lqos_api/src/api/network_json.rs:230` +- `../../lqos_api/src/api/network_json.rs:234` +- `../../lqos_api/src/api/network_json.rs:237` +- `../../lqos_api/src/api/network_json.rs:241` +- `../../lqos_api/src/api/network_json.rs:711` +- `../../lqos_api/src/api/network_json.rs:714` +- `../../lqos_api/src/api/network_json.rs:717` +- `../../lqos_api/src/api/network_json.rs:720` + +Short description: + +The API accepts site-speed values as `Option`, writes them into +`network.json`, and then narrows values to `f32` for live bus commands and queue +mapping reads. The reviewed code did not show finite/range validation before the +narrowing casts. + +Exposure / threat: + +The route is bearer-authenticated, so this is not unauthenticated input. A +credentialed caller can submit extremely large or nonsensical bandwidth values +that round or become non-finite when narrowed, depending on downstream handling. +The live command path may then diverge from the JSON value. I did not verify +whether downstream bus receivers reject these values. + +Recommended actions: + +- Validate site-speed request values as finite, positive, and within explicit + LibreQoS-supported Mbps bounds before writing JSON or sending live commands. +- Keep one numeric type across API, config, bus, and bakery code where practical. +- Add API tests for huge, negative, zero, fractional, and boundary bandwidth + values. + +#### API transit conversion can panic if RTT samples contain NaN + +Path: + +- `../../lqos_api/src/transit_types.rs:389` + +Short description: + +`NetworkJsonTransit::from` sorts RTT samples with +`partial_cmp(...).unwrap()`. `partial_cmp` returns `None` for NaN, which makes the +conversion panic if a NaN reaches the RTT vector. + +Exposure / threat: + +The current in-repo RTT producers reviewed here mostly derive RTT values from +durations, which should be finite. I did not find a clear external input path to +inject NaN into this vector, so this is marked reachability unknown. If a NaN +does enter the telemetry state, several API endpoints that serialize +`NetworkJsonTransit` can panic while preparing a response. + +Recommended actions: + +- Filter non-finite RTT samples before sorting, or sort with `f32::total_cmp`. +- Add a small conversion test with `[10.0, f32::NAN]` to prove the API response + path does not panic. + +### Observations / not findings + +- `../../lqos_api/src/web_security.rs:32` remains a confirmed malformed-header + panic, but it was already recorded in the network control-plane audit section. +- Unsafe callback paths such as `src/rust/lqos_heimdall/src/perf_interface.rs:70` + through `src/rust/lqos_heimdall/src/perf_interface.rs:77` check event size + before creating a byte slice. This is not counted as a new unsoundness finding. +- Broad Python exception handling exists in runtime files, including + `src/LibreQoS.py:2361`, `src/LibreQoS.py:2364`, and `src/LibreQoS.py:2549`. + The sampled paths are shaping-input tolerance or planner-weight fallback + behavior. They should be cleaned up for diagnosability, but I did not find a + concrete security impact in this stage. + +## Node Manager privacy, auth, and XSS audit + +Date: 2026-05-15 + +Scope: + +- Node Manager HTTP routing, static page routing, auth/session handling, + websocket request handling, local API data providers, frontend source under + `src/rust/lqosd/src/node_manager/js_build/src/`, and the templated static HTML + shell. +- Generated bundles, vendored JavaScript, static images, and source maps were + excluded from detailed XSS review unless a source file pointed to the behavior. +- This was a static source review. No browser exploit proof-of-concept was run. + +Review searches: + +- `rg -n "localStorage|sessionStorage|document\.cookie|innerHTML|outerHTML|insertAdjacentHTML|eval\(|Function\(|onclick=|onerror=|sanitize|DOMPurify|redact|redaction|redactable|allow_anonymous|auth_layer|route_layer|LoginResult|ReadOnly|Admin|Denied" src/rust/lqosd/src/node_manager docs/v2.0` +- `rg -n 'innerHTML\s*=.*(\+|`)|simpleRowHtml\(|href=.*\+|data-[^=]+=|textContent|innerText' src/rust/lqosd/src/node_manager/js_build/src --glob '*.js'` +- `rg -n "ShapedDevice|network_json|CircuitById|AllShapedDevices|NetworkJson|Search|UnknownIps|CircuitDirectory|device_name|circuit_name|mac|ipv4|ipv6|comment" src/rust/lqosd/src/node_manager` + +### Summary + +- Four confirmed findings are listed below: raw PII exposure in anonymous/read-only + surfaces, multiple DOM XSS sinks for operator/customer fields, JS-readable + session cookies, and dashboard-theme writes/XSS reachable by read-only users. +- I did not find a websocket path that serves data before the handshake completes: + the server sends `Hello`, requires `HelloReply`, rejects denied tokens, and + closes on other pre-handshake messages. +- Static HTML pages and `/local-api/*` are routed through `auth_layer`. The + fallback static file server remains a hardening concern, but the reviewed + current `static2` files are static assets or pages already listed in the + authenticated router. +- No session token or API key was found in `localStorage`. The higher-risk browser + storage issue is that the session cookie is readable by JavaScript so the + websocket client can copy it into the handshake. + +### Findings + +#### Anonymous/read-only mode exposes raw subscriber and topology data without server-side anonymization + +Paths: + +- `src/rust/lqos_config/src/authentication.rs:487` +- `src/rust/lqos_config/src/authentication.rs:498` +- `src/rust/lqosd/src/node_manager/auth.rs:421` +- `src/rust/lqosd/src/node_manager/auth.rs:431` +- `src/rust/lqosd/src/node_manager/ws.rs:2199` +- `src/rust/lqosd/src/node_manager/ws.rs:2207` +- `src/rust/lqosd/src/node_manager/local_api/config.rs:528` +- `src/rust/lqosd/src/node_manager/local_api/config.rs:541` +- `src/rust/lqosd/src/node_manager/local_api/shaped_devices_page.rs:118` +- `src/rust/lqosd/src/node_manager/local_api/shaped_devices_page.rs:123` +- `src/rust/lqosd/src/node_manager/local_api/shaped_devices_page.rs:131` +- `docs/v2.0/components.md:123` +- `docs/v2.0/components.md:126` +- `src/rust/lqosd/src/node_manager/js_build/src/helpers/redact.js:48` + +Short description: + +The documented anonymous option grants unauthenticated users `ReadOnly` access. +Read-only websocket requests include raw `network.json` and all shaped devices. +Those payloads include circuit names/IDs, device names/IDs, parent node names, +MACs, comments, and IPv4/IPv6 addresses. The current redaction feature is a +client-side display blur/font mode, documented as not modifying source data. + +Exposure / threat: + +Anonymous read-only mode is intended for demos, but demos are exactly where PII +redaction matters. Anyone who can reach a node with anonymous mode enabled can +request raw payloads directly over the websocket, regardless of whether the UI +redaction toggle is on. Read-only authenticated users also receive the same raw +data, so redaction is not a privacy boundary. + +Recommended actions: + +- Add a server-side anonymized/demo payload mode for anonymous access. Replace + subscriber/circuit/device names, IDs, IPs, MACs, and comments before sending + websocket responses. +- Consider field-level filtering for normal `ReadOnly` users where full + subscriber/device identifiers are not needed. +- Keep browser redaction as a screenshot tool, but do not rely on it for public + demo privacy. +- Add tests that anonymous mode cannot retrieve raw `AllShapedDevices`, + `NetworkJson`, circuit search, or circuit detail identifiers. + +#### Operator-controlled names reach `innerHTML` without escaping + +Paths: + +- `src/rust/lqosd/src/node_manager/js_build/src/template.js:797` +- `src/rust/lqosd/src/node_manager/js_build/src/template.js:799` +- `src/rust/lqosd/src/node_manager/js_build/src/template.js:801` +- `src/rust/lqosd/src/node_manager/js_build/src/circuit.js:2164` +- `src/rust/lqosd/src/node_manager/js_build/src/dashlets/top10_downloaders.js:97` +- `src/rust/lqosd/src/node_manager/js_build/src/dashlets/worst10_downloaders.js:97` +- `src/rust/lqosd/src/node_manager/js_build/src/dashlets/worst10_retransmits.js:101` +- `src/rust/lqosd/src/node_manager/js_build/src/config_users.js:101` +- `src/rust/lqosd/src/node_manager/js_build/src/config_users.js:104` +- `src/rust/lqosd/src/node_manager/js_build/src/helpers/builders.js:45` + +Short description: + +Several frontend paths concatenate circuit names, device names, site names, +dashboard/user fields, or other operator-controlled values into `innerHTML`. Some +nearby code has local `escapeHtml` helpers, but these sinks do not consistently +use them. `simpleRowHtml` is a reusable helper that writes its argument directly +to `td.innerHTML`. + +Exposure / threat: + +Circuit/device/site names can come from operator files or external integrations. +A malicious or compromised integration record containing HTML can execute script +when a user searches, opens a circuit page, views dashboard top lists, or opens +the user management page. Because the WebUI controls shaping and configuration, +stored XSS should be treated as a control-plane compromise path, not a cosmetic +bug. + +Recommended actions: + +- Replace these sinks with DOM construction and `textContent` for untrusted text. + Keep icons as separately created `` elements. +- Ban `simpleRowHtml` for untrusted data. Use `simpleRow` or an escaped helper + with a name that makes trust explicit. +- Escape attribute values separately from text nodes and validate URL protocols + before assigning links. +- Add frontend tests or a small DOM fixture using a circuit name like + `` to confirm it renders as text everywhere. + +#### WebUI session token is readable by JavaScript and copied into websocket auth + +Paths: + +- `src/rust/lqosd/src/node_manager/auth.rs:266` +- `src/rust/lqosd/src/node_manager/auth.rs:269` +- `src/rust/lqosd/src/node_manager/js_build/src/pubsub/ws.js:66` +- `src/rust/lqosd/src/node_manager/js_build/src/pubsub/ws.js:78` + +Short description: + +The `User-Token` session cookie is created with `SameSite=Lax`, but it is not +marked `HttpOnly` or `Secure`. The frontend websocket client reads +`document.cookie` and sends the session token in the websocket handshake. + +Exposure / threat: + +Any XSS in Node Manager can read and exfiltrate the signed session token. That +turns DOM injection into session theft, and an admin session can perform +configuration and shaping actions. `Secure` should also be set when the Caddy/TLS +option is active so the browser will not send the cookie over plain HTTP. + +Recommended actions: + +- Move websocket authentication to the server-side `Cookie` header during the + upgrade, or issue a short-lived websocket nonce that is not the durable session + token. +- Set `HttpOnly` on the session cookie once the websocket no longer needs + JavaScript cookie access. +- Set `Secure` whenever Node Manager is served through HTTPS/Caddy, with a + documented local-development exception if needed. +- Add an XSS regression test that verifies the session token is not available via + `document.cookie`. + +#### Read-only websocket users can write dashboard themes, enabling stored XSS + +Paths: + +- `src/rust/lqosd/src/node_manager/ws.rs:354` +- `src/rust/lqosd/src/node_manager/ws.rs:371` +- `src/rust/lqosd/src/node_manager/ws.rs:378` +- `src/rust/lqosd/src/node_manager/local_api/dashboard_themes.rs:35` +- `src/rust/lqosd/src/node_manager/local_api/dashboard_themes.rs:60` +- `src/rust/lqosd/src/node_manager/local_api/dashboard_themes.rs:97` +- `src/rust/lqosd/src/node_manager/js_build/src/lq_js_common/dashboard/dashboard.js:1024` + +Short description: + +The websocket dashboard-theme save/get/delete handlers do not check for +`LoginResult::Admin`. `dashboard_themes` only normalizes `/` and `\` in the theme +filename, stores the provided `name`, and later returns it for display. The saved +layout list writes `d.name` into `innerHTML`. + +Exposure / threat: + +Any authenticated read-only user, and any anonymous user when demo mode is +enabled, can save a dashboard layout with a malicious name. When an admin or +operator opens the saved-layout picker, that name can execute as stored XSS and +steal the JS-readable session token described above. + +Recommended actions: + +- Require `LoginResult::Admin` for `DashletSave` and `DashletDelete`, or document + and constrain why read-only users should be allowed to write shared layouts. +- Escape `d.name` when rendering saved layouts, preferably with `textContent`. +- Validate dashboard theme names server-side with an allowlist of printable + characters and a length limit. +- Add websocket tests for read-only denial and an XSS fixture for saved layout + names. + +### Hardening observations / not findings + +- `src/rust/lqosd/src/node_manager/ws.rs:311` through + `src/rust/lqosd/src/node_manager/ws.rs:329` require the websocket `HelloReply` + before processing normal requests. This review did not find a pre-handshake + data response. +- `src/rust/lqosd/src/node_manager/static_pages.rs:114` through + `src/rust/lqosd/src/node_manager/static_pages.rs:117` apply auth and templates + to the listed HTML pages, and `src/rust/lqosd/src/node_manager/local_api.rs:66` + applies `auth_layer` to `/local-api`. `src/rust/lqosd/src/node_manager/run.rs:85` + still has an unauthenticated fallback `ServeDir`; keep it restricted to static + assets and avoid adding data files or unlisted HTML pages under `static2`. +- `src/rust/lqosd/src/node_manager/local_api/config.rs:341` through + `src/rust/lqosd/src/node_manager/local_api/config.rs:349` require admin access + and redact integration secrets before returning the main config view. +- `src/rust/lqosd/src/node_manager/js_build/src/config_interface.js:4`, + `src/rust/lqosd/src/node_manager/js_build/src/config_interface.js:5`, + `src/rust/lqosd/src/node_manager/js_build/src/config_interface.js:376`, and + `src/rust/lqosd/src/node_manager/js_build/src/config_interface.js:424` store + network-mode drafts and pending operation IDs in `localStorage`. I did not find + credentials there, but interface names, VLANs, and operation IDs are + operationally sensitive on shared browsers. Prefer `sessionStorage` or clear + these keys on logout, confirm, and rollback. + +## Conclusion + +Overall security grade: **C**. + +LibreQoS is not starting from a bad place. The review found no confirmed +reachable CVE-class Rust dependency issue, the managed Caddy setup moves the +intended WebUI path toward loopback-backed TLS, Node Manager has a real +authentication/role model, eBPF packet parsing uses many explicit `data_end` +checks and bounded encapsulation loops, and several sensitive admin config values +are already redacted before they reach the browser. Those are meaningful +strengths, and they reduce the chance that the next release ships with a simple +dependency or parser-memory-safety failure. + +The weaknesses are concentrated around control-plane hardening and browser trust +boundaries. The biggest current risks are WebUI stored/DOM XSS, a JavaScript +readable session token, raw PII exposure in anonymous/read-only demo paths, an +API direct-listener path that can bypass the intended Caddy/TLS route if exposed, +request-time panics on malformed or stale inputs, and packet-path behavior that +can lose observability or consume CPU under malformed/high-cardinality bridged +traffic. + +The release posture should be: acceptable foundations, but not yet polished +security hygiene. Fixing the Node Manager XSS/session/anonymization issues and +the API/control-plane panic/listener issues would move the project materially +toward a **B**. Addressing the eBPF map-pressure/debug-logging findings, adding +rate limiting, and cleaning up numeric type-loss paths would make the security +state much more comfortable for routine release cadence. diff --git a/docs-es/v2.0/api-es.md b/docs-es/v2.0/api-es.md index 5019b63b..d9c9b9f5 100644 --- a/docs-es/v2.0/api-es.md +++ b/docs-es/v2.0/api-es.md @@ -16,6 +16,8 @@ Use Swagger en su nodo como referencia completa y superficie de prueba para la v - Puerto local directo de la API: `http://:9122/api-docs` - Si la WebUI usa HTTPS opcional con Caddy: `https:///api/v1/api-docs` +En sistemas que usan HTTPS con Caddy gestionado, deje el listener de la API en loopback y acceda a la API mediante `/api/v1/`. `LQOS_API_LISTEN` es un override avanzado para despliegues que exponen intencionalmente el listener de la API en forma directa; no lo configure en sistemas con Caddy gestionado salvo que la exposición directa, el firewall, TLS y los controles de acceso de operadores se hayan planificado juntos. + Use esta página como mapa de capacidades. Use Swagger para inventario completo de endpoints, esquemas request/response y pruebas en vivo. ¿Necesita definiciones de persistencia e impacto en runtime? Vea el [Glosario](glossary-es.md). diff --git a/docs-es/v2.0/https-caddy-es.md b/docs-es/v2.0/https-caddy-es.md index 91ea5d5e..98ada448 100644 --- a/docs-es/v2.0/https-caddy-es.md +++ b/docs-es/v2.0/https-caddy-es.md @@ -20,8 +20,15 @@ LibreQoS ofrece dos opciones simples: - Los operadores dejan de usar `http://tu_ip_del_shaper:9123` y pasan a usar `https://tu-hostname/` o `https://tu-ip-de-gestión/`. - LibreQoS mueve el listener de la WebUI a `127.0.0.1:9123`. - Caddy publica la WebUI y la documentación de la API por HTTPS. +- En modo Caddy gestionado, las solicitudes de API se envían a `127.0.0.1:9122`, de modo que la API queda detrás del mismo origen HTTPS que la WebUI. - Swagger pasa a `/api/v1/api-docs` en el mismo origen HTTPS de la WebUI. +## Overrides del listener de la API + +El modo Caddy gestionado espera que el servicio API permanezca en loopback y se alcance a través de Caddy en `/api/v1/`. + +`LQOS_API_LISTEN` es un override para despliegues avanzados. No lo configure en sistemas con Caddy gestionado, salvo que quiera exponer directamente el listener de la API y también haya planificado firewall, TLS y controles de acceso de operadores para ese listener directo. + ## Si usa el modo de certificado local Cuando deja el hostname vacío, el certificado raíz local de Caddy se guarda en el host LibreQoS en: diff --git a/docs/v2.0/api.md b/docs/v2.0/api.md index e1ee773f..fca6040f 100644 --- a/docs/v2.0/api.md +++ b/docs/v2.0/api.md @@ -16,6 +16,8 @@ Use Swagger on your node as the complete reference and test surface for your ins - Direct local API port: `http://:9122/api-docs` - If optional HTTPS with Caddy is enabled for the WebUI: `https:///api/v1/api-docs` +On systems using managed Caddy HTTPS, leave the API listener on loopback and access it through `/api/v1/`. `LQOS_API_LISTEN` is an advanced override for deployments that intentionally expose the API listener directly; do not set it on managed Caddy systems unless direct API exposure, firewalling, TLS, and operator access controls have been planned together. + Use this page as a capability map. Use Swagger for full endpoint inventory, request/response schemas, and live testing. Need definitions for persistence and runtime-impact terms? See the [Glossary](glossary.md). diff --git a/docs/v2.0/https-caddy.md b/docs/v2.0/https-caddy.md index 401c3950..8fed37e6 100644 --- a/docs/v2.0/https-caddy.md +++ b/docs/v2.0/https-caddy.md @@ -20,8 +20,15 @@ LibreQoS supports two simple choices: - Operators stop using `http://your_shaper_ip:9123` and instead use `https://your-hostname/` or `https://your-management-ip/`. - LibreQoS moves the WebUI listener to `127.0.0.1:9123`. - Caddy proxies the WebUI and API docs over HTTPS. +- Managed Caddy mode proxies API requests to `127.0.0.1:9122`, so the API stays behind the same HTTPS origin as the WebUI. - Swagger moves to `/api/v1/api-docs` on the same HTTPS origin as the WebUI. +## API Listen Overrides + +Managed Caddy mode expects the API service to remain on loopback and be reached through Caddy at `/api/v1/`. + +`LQOS_API_LISTEN` is an advanced deployment override. Do not set it on managed Caddy systems unless you intentionally want to expose the API listener directly and have also planned firewalling, TLS, and operator access controls for that direct listener. + ## If You Use The Local Certificate Mode When you leave the hostname blank, the Caddy local root certificate is stored on the LibreQoS host at: diff --git a/src/rust/Cargo.lock b/src/rust/Cargo.lock index e669ade8..1ae74993 100644 --- a/src/rust/Cargo.lock +++ b/src/rust/Cargo.lock @@ -678,7 +678,7 @@ dependencies = [ "aes-gcm", "base64", "percent-encoding", - "rand 0.8.5", + "rand 0.8.6", "subtle", "time", "version_check", @@ -2321,7 +2321,6 @@ dependencies = [ "parking_lot", "serde", "serde_json", - "tokio", "tracing", ] @@ -2345,7 +2344,7 @@ version = "0.1.0" dependencies = [ "allocative", "lqos_config", - "rand 0.8.5", + "rand 0.8.6", "serde", "surge-ping", "thiserror 1.0.69", @@ -2579,7 +2578,7 @@ dependencies = [ "once_cell", "parking_lot", "png", - "rand 0.8.5", + "rand 0.8.6", "reqwest", "serde", "serde_cbor", @@ -3366,9 +3365,9 @@ checksum = "f8dcc9c7d52a811697d2151c701e0d08956f92b0e24136cf4cf27b57a6a0d9bf" [[package]] name = "rand" -version = "0.8.5" +version = "0.8.6" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "34af8d1a0e25924bc5b7c43c079c942339d8f0a8b57c39049bef581b46327404" +checksum = "5ca0ecfa931c29007047d1bc58e623ab12e5590e8c7cc53200d5202b69266d8a" dependencies = [ "libc", "rand_chacha 0.3.1", @@ -4641,7 +4640,7 @@ dependencies = [ "httparse", "log", "native-tls", - "rand 0.8.5", + "rand 0.8.6", "sha1", "thiserror 1.0.69", "utf-8", diff --git a/src/rust/lqos_bus/src/bus/queue_data.rs b/src/rust/lqos_bus/src/bus/queue_data.rs index 7c0fd683..7cefae43 100644 --- a/src/rust/lqos_bus/src/bus/queue_data.rs +++ b/src/rust/lqos_bus/src/bus/queue_data.rs @@ -25,8 +25,8 @@ pub struct QueueStoreTransit { #[allow(missing_docs)] pub struct CakeDiffTransit { pub bytes: u64, - pub packets: u32, - pub qlen: u32, + pub packets: u64, + pub qlen: u64, pub tins: Vec, } @@ -34,10 +34,10 @@ pub struct CakeDiffTransit { #[allow(missing_docs)] pub struct CakeDiffTinTransit { pub sent_bytes: u64, - pub backlog_bytes: u32, - pub drops: u32, - pub marks: u32, - pub base_delay_us: u32, + pub backlog_bytes: u64, + pub drops: u64, + pub marks: u64, + pub base_delay_us: u64, } #[derive(Debug, Serialize, Deserialize, Clone, PartialEq, Default, Allocative)] @@ -46,21 +46,21 @@ pub struct CakeTransit { //pub handle: TcHandle, //pub parent: TcHandle, //pub bytes: u64, - //pub packets: u32, - //pub overlimits: u32, - //pub requeues: u32, - //pub backlog: u32, - //pub qlen: u32, - pub memory_used: u32, - //pub memory_limit: u32, - //pub capacity_estimate: u32, - //pub min_network_size: u16, - //pub max_network_size: u16, - //pub min_adj_size: u16, - //pub max_adj_size: u16, - //pub avg_hdr_offset: u16, + //pub packets: u64, + //pub overlimits: u64, + //pub requeues: u64, + //pub backlog: u64, + //pub qlen: u64, + pub memory_used: u64, + //pub memory_limit: u64, + //pub capacity_estimate: u64, + //pub min_network_size: u64, + //pub max_network_size: u64, + //pub min_adj_size: u64, + //pub max_adj_size: u64, + //pub avg_hdr_offset: u64, //pub tins: Vec, - //pub drops: u32, + //pub drops: u64, } /* @@ -88,23 +88,23 @@ pub struct CakeOptionsTransit { pub struct CakeTinTransit { //pub threshold_rate: u64, //pub sent_bytes: u64, - //pub backlog_bytes: u32, - //pub target_us: u32, - //pub interval_us: u32, - //pub peak_delay_us: u32, - //pub avg_delay_us: u32, - //pub base_delay_us: u32, - //pub sent_packets: u32, - //pub way_indirect_hits: u16, - //pub way_misses: u16, - //pub way_collisions: u16, - //pub drops: u32, - //pub ecn_marks: u32, - //pub ack_drops: u32, - //pub sparse_flows: u16, - //pub bulk_flows: u16, - //pub unresponsive_flows: u16, - //pub max_pkt_len: u16, - //pub flow_quantum: u16, + //pub backlog_bytes: u64, + //pub target_us: u64, + //pub interval_us: u64, + //pub peak_delay_us: u64, + //pub avg_delay_us: u64, + //pub base_delay_us: u64, + //pub sent_packets: u64, + //pub way_indirect_hits: u64, + //pub way_misses: u64, + //pub way_collisions: u64, + //pub drops: u64, + //pub ecn_marks: u64, + //pub ack_drops: u64, + //pub sparse_flows: u64, + //pub bulk_flows: u64, + //pub unresponsive_flows: u64, + //pub max_pkt_len: u64, + //pub flow_quantum: u64, } */ diff --git a/src/rust/lqos_config/src/authentication.rs b/src/rust/lqos_config/src/authentication.rs index ba457db5..7bd29fb3 100644 --- a/src/rust/lqos_config/src/authentication.rs +++ b/src/rust/lqos_config/src/authentication.rs @@ -110,8 +110,6 @@ pub struct WebUsers { #[serde(default = "default_auth_epoch")] auth_epoch: u64, #[serde(default)] - allow_unauthenticated_to_view: bool, - #[serde(default)] users: Vec, #[serde(skip)] base_path_override: Option, @@ -122,7 +120,6 @@ impl Default for WebUsers { Self { version: AUTH_FILE_VERSION, auth_epoch: INITIAL_AUTH_EPOCH, - allow_unauthenticated_to_view: false, users: Vec::new(), base_path_override: None, } @@ -240,7 +237,11 @@ impl WebUsers { Ok(()) } - fn migrate_if_needed(&mut self, loaded_from: &Path) -> Result<(), AuthenticationError> { + fn migrate_if_needed( + &mut self, + loaded_from: &Path, + removed_anonymous_setting_present: bool, + ) -> Result<(), AuthenticationError> { let current_path = Self::primary_path_for(&self.resolved_base_path()?); let loaded_from_legacy_path = loaded_from != current_path; let mut needs_rewrite = false; @@ -259,6 +260,10 @@ impl WebUsers { needs_rewrite = true; } + if removed_anonymous_setting_present { + needs_rewrite = true; + } + if needs_rewrite { self.bump_auth_epoch(); self.save_to_disk()?; @@ -279,12 +284,13 @@ impl WebUsers { error!("Unable to read auth file {:?}: {e}", path); AuthenticationError::UnableToRead })?; + let removed_anonymous_setting_present = auth_file_has_removed_anonymous_setting(&raw); let mut users: Self = toml_edit::de::from_str(&raw).map_err(|e| { error!("Unable to deserialize auth file {:?}: {e}", path); AuthenticationError::UnableToParse })?; users.base_path_override = Some(Self::default_base_path()?); - users.migrate_if_needed(&path)?; + users.migrate_if_needed(&path, removed_anonymous_setting_present)?; Ok(users) } else { let new_users = Self::default(); @@ -311,12 +317,13 @@ impl WebUsers { error!("Unable to read auth file {:?}: {e}", path); AuthenticationError::UnableToRead })?; + let removed_anonymous_setting_present = auth_file_has_removed_anonymous_setting(&raw); let mut users: Self = toml_edit::de::from_str(&raw).map_err(|e| { error!("Unable to deserialize auth file {:?}: {e}", path); AuthenticationError::UnableToParse })?; users.base_path_override = Some(base_path.to_path_buf()); - users.migrate_if_needed(&path)?; + users.migrate_if_needed(&path, removed_anonymous_setting_present)?; Ok(users) } else { let new_users = Self { @@ -483,21 +490,12 @@ impl WebUsers { pub fn get_users(&self) -> Vec { self.users.clone() } +} - /// Sets the "allow unauthenticated users" field. If true, - /// unauthenticated users gain read-only access. This is useful - /// for demonstration purposes. - pub fn allow_anonymous(&mut self, allow: bool) -> Result<(), AuthenticationError> { - self.allow_unauthenticated_to_view = allow; - self.bump_auth_epoch(); - self.save_to_disk()?; - Ok(()) - } - - /// Do we allow unauthenticated users to read site data? - pub fn do_we_allow_anonymous(&self) -> bool { - self.allow_unauthenticated_to_view - } +fn auth_file_has_removed_anonymous_setting(raw: &str) -> bool { + raw.parse::() + .map(|document| document.contains_key("allow_unauthenticated_to_view")) + .unwrap_or(false) } /// Errors that can occur while managing web-UI authentication. @@ -530,3 +528,42 @@ pub enum AuthenticationError { #[error("Invalid Login")] InvalidLogin, } + +#[cfg(test)] +mod tests { + use super::*; + use std::fs; + + fn temp_auth_dir(test_name: &str) -> PathBuf { + std::env::temp_dir().join(format!("libreqos-auth-{test_name}-{}", Uuid::new_v4())) + } + + #[test] + fn load_removes_legacy_anonymous_access_setting() { + let dir = temp_auth_dir("anonymous-setting"); + fs::create_dir_all(&dir).expect("create auth test directory"); + let path = dir.join(CURRENT_AUTH_FILE_NAME); + fs::write( + &path, + r#"version = 2 +auth_epoch = 1 +allow_unauthenticated_to_view = true + +[[users]] +username = "support" +password_hash = "legacy" +role = "ReadOnly" +"#, + ) + .expect("write auth file"); + + let users = WebUsers::load_or_create_in(&dir).expect("load auth file"); + assert_eq!(users.auth_epoch(), 2); + assert_eq!(users.get_users().len(), 1); + + let saved = fs::read_to_string(&path).expect("read migrated auth file"); + assert!(!saved.contains("allow_unauthenticated_to_view")); + + fs::remove_dir_all(&dir).expect("remove auth test directory"); + } +} diff --git a/src/rust/lqos_network_devices/Cargo.toml b/src/rust/lqos_network_devices/Cargo.toml index 6a91aa3c..747acc16 100644 --- a/src/rust/lqos_network_devices/Cargo.toml +++ b/src/rust/lqos_network_devices/Cargo.toml @@ -18,5 +18,4 @@ once_cell = { workspace = true } parking_lot = { workspace = true } serde = { workspace = true } serde_json = { workspace = true } -tokio = { workspace = true } tracing = { workspace = true } diff --git a/src/rust/lqos_probe/Cargo.toml b/src/rust/lqos_probe/Cargo.toml index 3903c0f4..db0509b5 100644 --- a/src/rust/lqos_probe/Cargo.toml +++ b/src/rust/lqos_probe/Cargo.toml @@ -11,5 +11,5 @@ thiserror = { workspace = true } tokio = { workspace = true } tracing = { workspace = true } lqos_config = { path = "../lqos_config" } -rand = "0.8.5" +rand = { version = "0.8.6", default-features = false, features = ["std", "std_rng"] } surge-ping = "0.8.1" diff --git a/src/rust/lqos_queue_tracker/src/queue_diff.rs b/src/rust/lqos_queue_tracker/src/queue_diff.rs index d5e3c207..3f251e66 100644 --- a/src/rust/lqos_queue_tracker/src/queue_diff.rs +++ b/src/rust/lqos_queue_tracker/src/queue_diff.rs @@ -64,18 +64,18 @@ fn queue_kind(queue: &QueueType) -> &'static str { #[derive(Serialize, Clone, Debug)] pub struct CakeDiff { pub bytes: u64, - pub packets: u32, - pub qlen: u32, + pub packets: u64, + pub qlen: u64, pub tins: Vec, } #[derive(Serialize, Clone, Debug)] pub struct CakeDiffTin { pub sent_bytes: u64, - pub backlog_bytes: u32, - pub drops: u32, - pub marks: u32, - pub base_delay_us: u32, + pub backlog_bytes: u64, + pub drops: u64, + pub marks: u64, + pub base_delay_us: u64, } fn cake_diff(previous: &QueueType, current: &QueueType) -> Result { @@ -113,10 +113,10 @@ pub enum QueueDiffError { #[derive(Serialize, Clone, Debug)] pub struct FqCodelDiff { pub bytes: u64, - pub packets: u32, - pub backlog: u32, - pub flows: u16, - pub ddrops: u32, + pub packets: u64, + pub backlog: u64, + pub flows: u64, + pub ddrops: u64, } fn fq_codel_diff(previous: &QueueType, current: &QueueType) -> Result { diff --git a/src/rust/lqos_queue_tracker/src/queue_types/tc_cake.rs b/src/rust/lqos_queue_tracker/src/queue_types/tc_cake.rs index 45ab338b..39e23b78 100644 --- a/src/rust/lqos_queue_tracker/src/queue_types/tc_cake.rs +++ b/src/rust/lqos_queue_tracker/src/queue_types/tc_cake.rs @@ -28,21 +28,21 @@ pub struct TcCake { pub(crate) parent: TcHandle, pub(crate) options: TcCakeOptions, pub(crate) bytes: u64, - pub(crate) packets: u32, - pub(crate) overlimits: u32, - pub(crate) requeues: u32, - pub(crate) backlog: u32, - pub(crate) qlen: u32, - pub(crate) memory_used: u32, - pub(crate) memory_limit: u32, - pub(crate) capacity_estimate: u32, - pub(crate) min_network_size: u16, - pub(crate) max_network_size: u16, - pub(crate) min_adj_size: u16, - pub(crate) max_adj_size: u16, - pub(crate) avg_hdr_offset: u16, + pub(crate) packets: u64, + pub(crate) overlimits: u64, + pub(crate) requeues: u64, + pub(crate) backlog: u64, + pub(crate) qlen: u64, + pub(crate) memory_used: u64, + pub(crate) memory_limit: u64, + pub(crate) capacity_estimate: u64, + pub(crate) min_network_size: u64, + pub(crate) max_network_size: u64, + pub(crate) min_adj_size: u64, + pub(crate) max_adj_size: u64, + pub(crate) avg_hdr_offset: u64, pub(crate) tins: Vec, - pub(crate) drops: u32, + pub(crate) drops: u64, } #[derive(Default, Clone, Debug, Serialize, Deserialize)] @@ -65,24 +65,24 @@ pub(crate) struct TcCakeOptions { pub(crate) struct TcCakeTin { pub(crate) threshold_rate: u64, pub(crate) sent_bytes: u64, - pub(crate) backlog_bytes: u32, - pub(crate) target_us: u32, - pub(crate) interval_us: u32, - pub(crate) peak_delay_us: u32, - pub(crate) avg_delay_us: u32, - pub(crate) base_delay_us: u32, - pub(crate) sent_packets: u32, - pub(crate) way_indirect_hits: u16, - pub(crate) way_misses: u16, - pub(crate) way_collisions: u16, - pub(crate) drops: u32, - pub(crate) ecn_marks: u32, - pub(crate) ack_drops: u32, - pub(crate) sparse_flows: u16, - pub(crate) bulk_flows: u16, - pub(crate) unresponsive_flows: u16, - pub(crate) max_pkt_len: u16, - pub(crate) flow_quantum: u16, + pub(crate) backlog_bytes: u64, + pub(crate) target_us: u64, + pub(crate) interval_us: u64, + pub(crate) peak_delay_us: u64, + pub(crate) avg_delay_us: u64, + pub(crate) base_delay_us: u64, + pub(crate) sent_packets: u64, + pub(crate) way_indirect_hits: u64, + pub(crate) way_misses: u64, + pub(crate) way_collisions: u64, + pub(crate) drops: u64, + pub(crate) ecn_marks: u64, + pub(crate) ack_drops: u64, + pub(crate) sparse_flows: u64, + pub(crate) bulk_flows: u64, + pub(crate) unresponsive_flows: u64, + pub(crate) max_pkt_len: u64, + pub(crate) flow_quantum: u64, } impl TcCake { @@ -99,22 +99,20 @@ impl TcCake { parse_tc_handle!(result.parent, value); } "bytes" => result.bytes = value.as_u64().unwrap_or(0), - "packets" => result.packets = value.as_u64().unwrap_or(0) as u32, - "overlimits" => result.overlimits = value.as_u64().unwrap_or(0) as u32, - "requeues" => result.requeues = value.as_u64().unwrap_or(0) as u32, - "backlog" => result.backlog = value.as_u64().unwrap_or(0) as u32, - "qlen" => result.qlen = value.as_u64().unwrap_or(0) as u32, - "memory_used" => result.memory_used = value.as_u64().unwrap_or(0) as u32, - "memory_limit" => result.memory_limit = value.as_u64().unwrap_or(0) as u32, - "capacity_estimate" => { - result.capacity_estimate = value.as_u64().unwrap_or(0) as u32 - } - "min_network_size" => result.min_network_size = value.as_u64().unwrap_or(0) as u16, - "max_network_size" => result.max_network_size = value.as_u64().unwrap_or(0) as u16, - "min_adj_size" => result.min_adj_size = value.as_u64().unwrap_or(0) as u16, - "max_adj_size" => result.max_adj_size = value.as_u64().unwrap_or(0) as u16, - "avg_hdr_offset" => result.avg_hdr_offset = value.as_u64().unwrap_or(0) as u16, - "drops" => result.drops = value.as_u64().unwrap_or(0) as u32, + "packets" => result.packets = value.as_u64().unwrap_or(0), + "overlimits" => result.overlimits = value.as_u64().unwrap_or(0), + "requeues" => result.requeues = value.as_u64().unwrap_or(0), + "backlog" => result.backlog = value.as_u64().unwrap_or(0), + "qlen" => result.qlen = value.as_u64().unwrap_or(0), + "memory_used" => result.memory_used = value.as_u64().unwrap_or(0), + "memory_limit" => result.memory_limit = value.as_u64().unwrap_or(0), + "capacity_estimate" => result.capacity_estimate = value.as_u64().unwrap_or(0), + "min_network_size" => result.min_network_size = value.as_u64().unwrap_or(0), + "max_network_size" => result.max_network_size = value.as_u64().unwrap_or(0), + "min_adj_size" => result.min_adj_size = value.as_u64().unwrap_or(0), + "max_adj_size" => result.max_adj_size = value.as_u64().unwrap_or(0), + "avg_hdr_offset" => result.avg_hdr_offset = value.as_u64().unwrap_or(0), + "drops" => result.drops = value.as_u64().unwrap_or(0), "options" => result.options = TcCakeOptions::from_json(value)?, "tins" => { if let Value::Array(array) = value { @@ -183,36 +181,28 @@ impl TcCakeTin { match key.as_str() { "threshold_rate" => result.threshold_rate = value.as_u64().unwrap_or(0), "sent_bytes" => result.sent_bytes = value.as_u64().unwrap_or(0), - "backlog_bytes" => { - result.backlog_bytes = value.as_u64().unwrap_or(0) as u32 - } - "target_us" => result.target_us = value.as_u64().unwrap_or(0) as u32, - "interval_us" => result.interval_us = value.as_u64().unwrap_or(0) as u32, - "peak_delay_us" => { - result.peak_delay_us = value.as_u64().unwrap_or(0) as u32 - } - "avg_delay_us" => result.avg_delay_us = value.as_u64().unwrap_or(0) as u32, - "base_delay_us" => { - result.base_delay_us = value.as_u64().unwrap_or(0) as u32 - } - "sent_packets" => result.sent_packets = value.as_u64().unwrap_or(0) as u32, + "backlog_bytes" => result.backlog_bytes = value.as_u64().unwrap_or(0), + "target_us" => result.target_us = value.as_u64().unwrap_or(0), + "interval_us" => result.interval_us = value.as_u64().unwrap_or(0), + "peak_delay_us" => result.peak_delay_us = value.as_u64().unwrap_or(0), + "avg_delay_us" => result.avg_delay_us = value.as_u64().unwrap_or(0), + "base_delay_us" => result.base_delay_us = value.as_u64().unwrap_or(0), + "sent_packets" => result.sent_packets = value.as_u64().unwrap_or(0), "way_indirect_hits" => { - result.way_indirect_hits = value.as_u64().unwrap_or(0) as u16 + result.way_indirect_hits = value.as_u64().unwrap_or(0) } - "way_misses" => result.way_misses = value.as_u64().unwrap_or(0) as u16, - "way_collisions" => { - result.way_collisions = value.as_u64().unwrap_or(0) as u16 - } - "drops" => result.drops = value.as_u64().unwrap_or(0) as u32, - "ecn_mark" => result.ecn_marks = value.as_u64().unwrap_or(0) as u32, - "ack_drops" => result.ack_drops = value.as_u64().unwrap_or(0) as u32, - "sparse_flows" => result.sparse_flows = value.as_u64().unwrap_or(0) as u16, - "bulk_flows" => result.bulk_flows = value.as_u64().unwrap_or(0) as u16, + "way_misses" => result.way_misses = value.as_u64().unwrap_or(0), + "way_collisions" => result.way_collisions = value.as_u64().unwrap_or(0), + "drops" => result.drops = value.as_u64().unwrap_or(0), + "ecn_mark" => result.ecn_marks = value.as_u64().unwrap_or(0), + "ack_drops" => result.ack_drops = value.as_u64().unwrap_or(0), + "sparse_flows" => result.sparse_flows = value.as_u64().unwrap_or(0), + "bulk_flows" => result.bulk_flows = value.as_u64().unwrap_or(0), "unresponsive_flows" => { - result.unresponsive_flows = value.as_u64().unwrap_or(0) as u16 + result.unresponsive_flows = value.as_u64().unwrap_or(0) } - "max_pkt_len" => result.max_pkt_len = value.as_u64().unwrap_or(0) as u16, - "flow_quantum" => result.flow_quantum = value.as_u64().unwrap_or(0) as u16, + "max_pkt_len" => result.max_pkt_len = value.as_u64().unwrap_or(0), + "flow_quantum" => result.flow_quantum = value.as_u64().unwrap_or(0), _ => { info!("Unknown entry in tc-cake-tin json decoder: {key}"); } @@ -358,3 +348,51 @@ impl TcCakeTin { }, */ + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn cake_parser_preserves_large_kernel_counters() { + let value = serde_json::json!({ + "kind": "cake", + "handle": "9cb1:", + "parent": "3:205", + "options": {}, + "bytes": u64::from(u32::MAX) + 11, + "packets": u64::from(u32::MAX) + 12, + "drops": u64::from(u32::MAX) + 13, + "overlimits": u64::from(u32::MAX) + 14, + "requeues": u64::from(u32::MAX) + 15, + "backlog": u64::from(u32::MAX) + 16, + "qlen": u64::from(u32::MAX) + 17, + "memory_used": u64::from(u32::MAX) + 18, + "memory_limit": u64::from(u32::MAX) + 19, + "tins": [{ + "sent_bytes": u64::from(u32::MAX) + 20, + "backlog_bytes": u64::from(u32::MAX) + 21, + "sent_packets": u64::from(u32::MAX) + 22, + "way_indirect_hits": u64::from(u16::MAX) + 23, + "drops": u64::from(u32::MAX) + 24, + "ecn_mark": u64::from(u32::MAX) + 25, + "ack_drops": u64::from(u32::MAX) + 26, + "sparse_flows": u64::from(u16::MAX) + 27 + }] + }); + let Value::Object(map) = value else { + panic!("test fixture should be a JSON object"); + }; + + let parsed = TcCake::from_json(&map).expect("cake fixture should parse"); + + assert_eq!(parsed.packets, u64::from(u32::MAX) + 12); + assert_eq!(parsed.drops, u64::from(u32::MAX) + 13); + assert_eq!(parsed.backlog, u64::from(u32::MAX) + 16); + assert_eq!(parsed.memory_used, u64::from(u32::MAX) + 18); + assert_eq!(parsed.tins[0].sent_packets, u64::from(u32::MAX) + 22); + assert_eq!(parsed.tins[0].way_indirect_hits, u64::from(u16::MAX) + 23); + assert_eq!(parsed.tins[0].ecn_marks, u64::from(u32::MAX) + 25); + assert_eq!(parsed.tins[0].sparse_flows, u64::from(u16::MAX) + 27); + } +} diff --git a/src/rust/lqos_queue_tracker/src/queue_types/tc_fq_codel.rs b/src/rust/lqos_queue_tracker/src/queue_types/tc_fq_codel.rs index 40dea0e0..a2d986a7 100644 --- a/src/rust/lqos_queue_tracker/src/queue_types/tc_fq_codel.rs +++ b/src/rust/lqos_queue_tracker/src/queue_types/tc_fq_codel.rs @@ -18,30 +18,30 @@ pub struct TcFqCodel { pub(crate) parent: TcHandle, pub(crate) options: TcFqCodelOptions, pub(crate) bytes: u64, - pub(crate) packets: u32, // FIXME - for long term data we have to worry about wrapping - pub(crate) drops: u32, - pub(crate) overlimits: u32, - pub(crate) requeues: u32, - pub(crate) backlog: u32, - pub(crate) qlen: u32, - pub(crate) maxpacket: u16, - pub(crate) drop_overlimit: u32, - pub(crate) new_flow_count: u32, - pub(crate) ecn_mark: u32, - pub(crate) new_flows_len: u16, - pub(crate) old_flows_len: u16, + pub(crate) packets: u64, + pub(crate) drops: u64, + pub(crate) overlimits: u64, + pub(crate) requeues: u64, + pub(crate) backlog: u64, + pub(crate) qlen: u64, + pub(crate) maxpacket: u64, + pub(crate) drop_overlimit: u64, + pub(crate) new_flow_count: u64, + pub(crate) ecn_mark: u64, + pub(crate) new_flows_len: u64, + pub(crate) old_flows_len: u64, } #[derive(Default, Clone, Debug, Serialize)] pub(crate) struct TcFqCodelOptions { - pub(crate) limit: u32, - pub(crate) flows: u16, - pub(crate) quantum: u16, + pub(crate) limit: u64, + pub(crate) flows: u64, + pub(crate) quantum: u64, pub(crate) target: u64, // FIXME target and interval within fq_codel are scaled to ns >> 1024 pub(crate) interval: u64, // tc scales them back up to us. Ideally ns would make sense throughout. - pub(crate) memory_limit: u32, + pub(crate) memory_limit: u64, pub(crate) ecn: bool, - pub(crate) drop_batch: u16, // FIXME CE_threshold is presently missing from the parser + pub(crate) drop_batch: u64, // FIXME CE_threshold is presently missing from the parser } impl TcFqCodel { @@ -58,18 +58,18 @@ impl TcFqCodel { parse_tc_handle!(result.parent, value); } "bytes" => result.bytes = value.as_u64().unwrap_or(0), - "packets" => result.packets = value.as_u64().unwrap_or(0) as u32, - "drops" => result.drops = value.as_u64().unwrap_or(0) as u32, - "overlimits" => result.overlimits = value.as_u64().unwrap_or(0) as u32, - "requeues" => result.requeues = value.as_u64().unwrap_or(0) as u32, - "backlog" => result.backlog = value.as_u64().unwrap_or(0) as u32, - "qlen" => result.qlen = value.as_u64().unwrap_or(0) as u32, - "maxpacket" => result.maxpacket = value.as_u64().unwrap_or(0) as u16, - "drop_overlimit" => result.drop_overlimit = value.as_u64().unwrap_or(0) as u32, - "new_flow_count" => result.new_flow_count = value.as_u64().unwrap_or(0) as u32, - "ecn_mark" => result.ecn_mark = value.as_u64().unwrap_or(0) as u32, - "new_flows_len" => result.new_flows_len = value.as_u64().unwrap_or(0) as u16, - "old_flows_len" => result.old_flows_len = value.as_u64().unwrap_or(0) as u16, + "packets" => result.packets = value.as_u64().unwrap_or(0), + "drops" => result.drops = value.as_u64().unwrap_or(0), + "overlimits" => result.overlimits = value.as_u64().unwrap_or(0), + "requeues" => result.requeues = value.as_u64().unwrap_or(0), + "backlog" => result.backlog = value.as_u64().unwrap_or(0), + "qlen" => result.qlen = value.as_u64().unwrap_or(0), + "maxpacket" => result.maxpacket = value.as_u64().unwrap_or(0), + "drop_overlimit" => result.drop_overlimit = value.as_u64().unwrap_or(0), + "new_flow_count" => result.new_flow_count = value.as_u64().unwrap_or(0), + "ecn_mark" => result.ecn_mark = value.as_u64().unwrap_or(0), + "new_flows_len" => result.new_flows_len = value.as_u64().unwrap_or(0), + "old_flows_len" => result.old_flows_len = value.as_u64().unwrap_or(0), "options" => result.options = TcFqCodelOptions::from_json(value)?, "kind" => {} _ => { @@ -88,14 +88,14 @@ impl TcFqCodelOptions { let mut result = Self::default(); for (key, value) in map.iter() { match key.as_str() { - "limit" => result.limit = value.as_u64().unwrap_or(0) as u32, - "flows" => result.flows = value.as_u64().unwrap_or(0) as u16, - "quantum" => result.quantum = value.as_u64().unwrap_or(0) as u16, + "limit" => result.limit = value.as_u64().unwrap_or(0), + "flows" => result.flows = value.as_u64().unwrap_or(0), + "quantum" => result.quantum = value.as_u64().unwrap_or(0), "target" => result.target = value.as_u64().unwrap_or(0), "interval" => result.interval = value.as_u64().unwrap_or(0), - "memory_limit" => result.memory_limit = value.as_u64().unwrap_or(0) as u32, + "memory_limit" => result.memory_limit = value.as_u64().unwrap_or(0), "ecn" => result.ecn = value.as_bool().unwrap_or(false), - "drop_batch" => result.drop_batch = value.as_u64().unwrap_or(0) as u16, + "drop_batch" => result.drop_batch = value.as_u64().unwrap_or(0), _ => { info!("Unknown entry in tc-codel-options json decoder: {key}"); } @@ -107,3 +107,50 @@ impl TcFqCodelOptions { } } } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn fq_codel_parser_preserves_large_kernel_counters() { + let value = serde_json::json!({ + "kind": "fq_codel", + "handle": "0:", + "parent": "7fff:a", + "options": { + "limit": u64::from(u32::MAX) + 1, + "flows": u64::from(u16::MAX) + 2, + "quantum": u64::from(u16::MAX) + 3, + "memory_limit": u64::from(u32::MAX) + 4, + "drop_batch": u64::from(u16::MAX) + 5 + }, + "bytes": u64::from(u32::MAX) + 6, + "packets": u64::from(u32::MAX) + 7, + "drops": u64::from(u32::MAX) + 8, + "overlimits": u64::from(u32::MAX) + 9, + "requeues": u64::from(u32::MAX) + 10, + "backlog": u64::from(u32::MAX) + 11, + "qlen": u64::from(u32::MAX) + 12, + "drop_overlimit": u64::from(u32::MAX) + 13, + "new_flow_count": u64::from(u32::MAX) + 14, + "ecn_mark": u64::from(u32::MAX) + 15, + "new_flows_len": u64::from(u16::MAX) + 16, + "old_flows_len": u64::from(u16::MAX) + 17 + }); + let Value::Object(map) = value else { + panic!("test fixture should be a JSON object"); + }; + + let parsed = TcFqCodel::from_json(&map).expect("fq_codel fixture should parse"); + + assert_eq!(parsed.options.flows, u64::from(u16::MAX) + 2); + assert_eq!(parsed.options.memory_limit, u64::from(u32::MAX) + 4); + assert_eq!(parsed.packets, u64::from(u32::MAX) + 7); + assert_eq!(parsed.drops, u64::from(u32::MAX) + 8); + assert_eq!(parsed.backlog, u64::from(u32::MAX) + 11); + assert_eq!(parsed.drop_overlimit, u64::from(u32::MAX) + 13); + assert_eq!(parsed.ecn_mark, u64::from(u32::MAX) + 15); + assert_eq!(parsed.new_flows_len, u64::from(u16::MAX) + 16); + } +} diff --git a/src/rust/lqos_queue_tracker/src/queue_types/tc_htb.rs b/src/rust/lqos_queue_tracker/src/queue_types/tc_htb.rs index 1bd1bf08..fce9a7d6 100644 --- a/src/rust/lqos_queue_tracker/src/queue_types/tc_htb.rs +++ b/src/rust/lqos_queue_tracker/src/queue_types/tc_htb.rs @@ -15,21 +15,21 @@ pub struct TcHtb { handle: TcHandle, parent: TcHandle, bytes: u64, - packets: u32, - drops: u32, - overlimits: u32, - requeues: u32, - backlog: u32, - qlen: u32, + packets: u64, + drops: u64, + overlimits: u64, + requeues: u64, + backlog: u64, + qlen: u64, options: TcHtbOptions, } #[derive(Default, Clone, Debug, Serialize)] struct TcHtbOptions { default: TcHandle, - r2q: u32, - direct_qlen: u32, - direct_packets_stat: u32, + r2q: u64, + direct_qlen: u64, + direct_packets_stat: u64, } impl TcHtb { @@ -46,12 +46,12 @@ impl TcHtb { parse_tc_handle!(result.parent, value); } "bytes" => result.bytes = value.as_u64().unwrap_or(0), - "packets" => result.packets = value.as_u64().unwrap_or(0) as u32, - "drops" => result.drops = value.as_u64().unwrap_or(0) as u32, - "overlimits" => result.overlimits = value.as_u64().unwrap_or(0) as u32, - "requeues" => result.requeues = value.as_u64().unwrap_or(0) as u32, - "backlog" => result.backlog = value.as_u64().unwrap_or(0) as u32, - "qlen" => result.qlen = value.as_u64().unwrap_or(0) as u32, + "packets" => result.packets = value.as_u64().unwrap_or(0), + "drops" => result.drops = value.as_u64().unwrap_or(0), + "overlimits" => result.overlimits = value.as_u64().unwrap_or(0), + "requeues" => result.requeues = value.as_u64().unwrap_or(0), + "backlog" => result.backlog = value.as_u64().unwrap_or(0), + "qlen" => result.qlen = value.as_u64().unwrap_or(0), "options" => result.options = TcHtbOptions::from_json(value)?, "kind" => {} _ => { @@ -70,14 +70,14 @@ impl TcHtbOptions { let mut result = Self::default(); for (key, value) in map.iter() { match key.as_str() { - "r2q" => result.r2q = value.as_u64().unwrap_or(0) as u32, + "r2q" => result.r2q = value.as_u64().unwrap_or(0), "default" => { parse_tc_handle!(result.default, value); } "direct_packets_stat" => { - result.direct_packets_stat = value.as_u64().unwrap_or(0) as u32 + result.direct_packets_stat = value.as_u64().unwrap_or(0) } - "direct_qlen" => result.direct_qlen = value.as_u64().unwrap_or(0) as u32, + "direct_qlen" => result.direct_qlen = value.as_u64().unwrap_or(0), _ => { info!("Unknown entry in tc-HTB json decoder: {key}"); } @@ -89,3 +89,39 @@ impl TcHtbOptions { } } } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn htb_parser_preserves_large_kernel_counters() { + let value = serde_json::json!({ + "kind": "htb", + "handle": "2:", + "parent": "7fff:2", + "options": { + "direct_packets_stat": u64::from(u32::MAX) + 1, + "direct_qlen": u64::from(u32::MAX) + 2 + }, + "bytes": u64::from(u32::MAX) + 3, + "packets": u64::from(u32::MAX) + 4, + "drops": u64::from(u32::MAX) + 5, + "overlimits": u64::from(u32::MAX) + 6, + "requeues": u64::from(u32::MAX) + 7, + "backlog": u64::from(u32::MAX) + 8, + "qlen": u64::from(u32::MAX) + 9 + }); + let Value::Object(map) = value else { + panic!("test fixture should be a JSON object"); + }; + + let parsed = TcHtb::from_json(&map).expect("htb fixture should parse"); + + assert_eq!(parsed.options.direct_packets_stat, u64::from(u32::MAX) + 1); + assert_eq!(parsed.options.direct_qlen, u64::from(u32::MAX) + 2); + assert_eq!(parsed.packets, u64::from(u32::MAX) + 4); + assert_eq!(parsed.drops, u64::from(u32::MAX) + 5); + assert_eq!(parsed.backlog, u64::from(u32::MAX) + 8); + } +} diff --git a/src/rust/lqos_queue_tracker/src/queue_types/tc_mq.rs b/src/rust/lqos_queue_tracker/src/queue_types/tc_mq.rs index efd49f95..d0869d6a 100644 --- a/src/rust/lqos_queue_tracker/src/queue_types/tc_mq.rs +++ b/src/rust/lqos_queue_tracker/src/queue_types/tc_mq.rs @@ -14,12 +14,12 @@ pub struct TcMultiQueue { handle: TcHandle, root: bool, bytes: u64, - packets: u32, // FIXME These can overflow in older linuxes - drops: u32, - overlimits: u32, - requeues: u32, // what does requeues really mean? - backlog: u32, - qlen: u32, + packets: u64, + drops: u64, + overlimits: u64, + requeues: u64, // what does requeues really mean? + backlog: u64, + qlen: u64, } impl TcMultiQueue { @@ -34,12 +34,12 @@ impl TcMultiQueue { } "root" => result.root = value.as_bool().unwrap_or(false), "bytes" => result.bytes = value.as_u64().unwrap_or(0), - "packets" => result.packets = value.as_u64().unwrap_or(0) as u32, - "drops" => result.drops = value.as_u64().unwrap_or(0) as u32, - "overlimits" => result.overlimits = value.as_u64().unwrap_or(0) as u32, - "requeues" => result.requeues = value.as_u64().unwrap_or(0) as u32, - "backlog" => result.backlog = value.as_u64().unwrap_or(0) as u32, - "qlen" => result.qlen = value.as_u64().unwrap_or(0) as u32, + "packets" => result.packets = value.as_u64().unwrap_or(0), + "drops" => result.drops = value.as_u64().unwrap_or(0), + "overlimits" => result.overlimits = value.as_u64().unwrap_or(0), + "requeues" => result.requeues = value.as_u64().unwrap_or(0), + "backlog" => result.backlog = value.as_u64().unwrap_or(0), + "qlen" => result.qlen = value.as_u64().unwrap_or(0), "kind" => {} "options" => {} _ => { @@ -50,3 +50,35 @@ impl TcMultiQueue { Ok(result) } } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn mq_parser_preserves_large_kernel_counters() { + let value = serde_json::json!({ + "kind": "mq", + "handle": "7fff:", + "root": true, + "options": {}, + "bytes": u64::from(u32::MAX) + 1, + "packets": u64::from(u32::MAX) + 2, + "drops": u64::from(u32::MAX) + 3, + "overlimits": u64::from(u32::MAX) + 4, + "requeues": u64::from(u32::MAX) + 5, + "backlog": u64::from(u32::MAX) + 6, + "qlen": u64::from(u32::MAX) + 7 + }); + let Value::Object(map) = value else { + panic!("test fixture should be a JSON object"); + }; + + let parsed = TcMultiQueue::from_json(&map).expect("mq fixture should parse"); + + assert_eq!(parsed.packets, u64::from(u32::MAX) + 2); + assert_eq!(parsed.drops, u64::from(u32::MAX) + 3); + assert_eq!(parsed.backlog, u64::from(u32::MAX) + 6); + assert_eq!(parsed.qlen, u64::from(u32::MAX) + 7); + } +} diff --git a/src/rust/lqos_queue_tracker/src/tracking/mod.rs b/src/rust/lqos_queue_tracker/src/tracking/mod.rs index 8904bb31..40238144 100644 --- a/src/rust/lqos_queue_tracker/src/tracking/mod.rs +++ b/src/rust/lqos_queue_tracker/src/tracking/mod.rs @@ -127,12 +127,12 @@ fn connect_queues_to_circuit(structure: &[QueueNode], queues: &[QueueType]) -> V //println!("It matched!"); if let Some(circuit_hash) = &s.circuit_hash { //println!("Circuit hash: {:?}", circuit_hash); - let marks: u32 = cake.tins.iter().map(|tin| tin.ecn_marks).sum(); + let marks: u64 = cake.tins.iter().map(|tin| tin.ecn_marks).sum(); if cake.drops > 0 || marks > 0 { return Some(TrackedQueue { circuit_hash: *circuit_hash, - drops: cake.drops as u64, - marks: marks as u64, + drops: cake.drops, + marks, }); } } @@ -157,12 +157,12 @@ fn connect_queues_to_circuit_up( .find(|s| s.up_class_major == major as u32 && s.class_minor == minor as u32) && let Some(circuit_hash) = &s.circuit_hash { - let marks: u32 = cake.tins.iter().map(|tin| tin.ecn_marks).sum(); + let marks: u64 = cake.tins.iter().map(|tin| tin.ecn_marks).sum(); if cake.drops > 0 || marks > 0 { return Some(TrackedQueue { circuit_hash: *circuit_hash, - drops: cake.drops as u64, - marks: marks as u64, + drops: cake.drops, + marks, }); } } diff --git a/src/rust/lqos_setup/src/ssl.rs b/src/rust/lqos_setup/src/ssl.rs index cf1dd7a7..7c4980be 100644 --- a/src/rust/lqos_setup/src/ssl.rs +++ b/src/rust/lqos_setup/src/ssl.rs @@ -102,7 +102,7 @@ pub fn ssl_status(config: &Config, preferred_host: Option<&str>) -> SslStatus { /// - updates `/etc/lqos.conf` /// - runs the Caddy installer helper if needed /// - writes `/etc/caddy/Caddyfile` -/// - schedules a delayed service switch that restarts `caddy.service` and `lqosd.service` +/// - schedules a delayed service switch that restarts `caddy.service`, `lqosd.service`, and `lqos_api.service` pub fn enable_runtime_ssl( external_hostname: Option, preferred_host: Option<&str>, @@ -149,7 +149,7 @@ pub fn enable_runtime_ssl( /// - updates `/etc/lqos.conf` /// - removes the managed `/etc/caddy/Caddyfile` /// - runs the Caddy disable helper when LibreQoS owns that setup -/// - schedules a delayed service switch that restarts `lqosd.service` +/// - schedules a delayed service switch that restarts `lqosd.service` and `lqos_api.service` pub fn disable_runtime_ssl(preferred_host: Option<&str>) -> Result { let existing = (*lqos_config::load_config()?).clone(); let ssl = existing @@ -556,7 +556,7 @@ fn render_managed_caddyfile(plan: &SslRuntimePlan) -> String { } fn enable_runtime_command() -> &'static str { - "systemctl enable caddy.service && systemctl restart caddy.service && systemctl restart lqosd.service" + "systemctl enable caddy.service && systemctl restart caddy.service && systemctl restart lqosd.service && systemctl restart lqos_api.service" } fn enable_setup_command() -> &'static str { @@ -564,7 +564,7 @@ fn enable_setup_command() -> &'static str { } fn disable_runtime_command() -> &'static str { - "systemctl stop caddy.service || true; systemctl disable caddy.service || true; systemctl restart lqosd.service" + "systemctl stop caddy.service || true; systemctl disable caddy.service || true; systemctl restart lqosd.service && systemctl restart lqos_api.service" } fn schedule_delayed_runtime_switch(command: &str) -> Result<()> { @@ -582,7 +582,9 @@ fn schedule_delayed_runtime_switch(command: &str) -> Result<()> { .stdout(Stdio::null()) .stderr(Stdio::null()) .status() - .with_context(|| format!("Unable to invoke {SYSTEMD_RUN_BIN} for SSL service switch"))?; + .with_context(|| { + format!("Unable to invoke {SYSTEMD_RUN_BIN} for SSL service switch") + })?; if !status.success() { bail!("Unable to schedule SSL service switch via systemd-run: {status}"); } @@ -647,6 +649,26 @@ mod tests { assert_eq!(caddyfile, expected); } + #[test] + fn managed_caddyfile_keeps_api_and_webui_upstreams_loopback() { + let plan = build_runtime_plan(Some("libreqos.example.com".to_string()), None); + let caddyfile = render_managed_caddyfile(&plan); + + assert!(caddyfile.contains(&format!("reverse_proxy {API_UPSTREAM}"))); + assert!(caddyfile.contains(&format!("reverse_proxy {WEB_UPSTREAM}"))); + for exposed_upstream in [ + "reverse_proxy :9122", + "reverse_proxy 0.0.0.0:9122", + "reverse_proxy [::]:9122", + "reverse_proxy :::9122", + ] { + assert!( + !caddyfile.contains(exposed_upstream), + "managed Caddy should not proxy API traffic through {exposed_upstream}" + ); + } + } + #[test] fn internal_ca_caddyfile_adds_tls_internal() { let plan = build_runtime_plan(None, None); @@ -738,4 +760,30 @@ mod tests { .enabled ); } + + #[test] + fn runtime_enable_restarts_api_after_caddy_mode_change() { + let command = super::enable_runtime_command(); + + assert!(command.contains("systemctl restart caddy.service")); + assert!(command.contains("systemctl restart lqosd.service")); + assert!(command.contains("systemctl restart lqos_api.service")); + assert!( + command.find("systemctl restart lqosd.service") + < command.find("systemctl restart lqos_api.service") + ); + } + + #[test] + fn runtime_disable_restarts_api_after_caddy_mode_change() { + let command = super::disable_runtime_command(); + + assert!(command.contains("systemctl disable caddy.service")); + assert!(command.contains("systemctl restart lqosd.service")); + assert!(command.contains("systemctl restart lqos_api.service")); + assert!( + command.find("systemctl restart lqosd.service") + < command.find("systemctl restart lqos_api.service") + ); + } } diff --git a/src/rust/lqosd/Cargo.toml b/src/rust/lqosd/Cargo.toml index 4f894e5b..132a5a7b 100644 --- a/src/rust/lqosd/Cargo.toml +++ b/src/rust/lqosd/Cargo.toml @@ -48,10 +48,10 @@ zerocopy = { workspace = true } fxhash = "0.2.1" axum = { version = "0.7.7", features = ["ws"] } axum-extra = { version = "0.9.4", features = ["cookie", "cookie-private"] } -tower-http = { version = "0.6.1", features = ["fs", "cors"] } +tower-http = { version = "0.6.1", features = ["fs"] } strum = { version = "0.26.3", features = ["derive"] } default-net = { workspace = true } -rand = "0.8.5" +rand = { version = "0.8.6", default-features = false, features = ["std", "std_rng"] } serde_cbor = { workspace = true } timerfd = { workspace = true } crossbeam-channel = { workspace = true } diff --git a/src/rust/lqosd/src/main.rs b/src/rust/lqosd/src/main.rs index 29b4ebd1..f6af8e9f 100644 --- a/src/rust/lqosd/src/main.rs +++ b/src/rust/lqosd/src/main.rs @@ -1116,6 +1116,10 @@ fn handle_bus_requests(requests: &[BusRequest], responses: &mut Vec } BusRequest::GetAsnFlowTimeline { asn } => { let data = node_manager::flow_timeline_data(*asn) + .unwrap_or_else(|err| { + warn!("Unable to build ASN flow timeline: {err}"); + Vec::new() + }) .into_iter() .map(flow_timeline_to_bus) .collect(); @@ -1123,6 +1127,10 @@ fn handle_bus_requests(requests: &[BusRequest], responses: &mut Vec } BusRequest::GetCountryFlowTimeline { iso_code } => { let data = node_manager::country_timeline_data(iso_code) + .unwrap_or_else(|err| { + warn!("Unable to build country flow timeline: {err}"); + Vec::new() + }) .into_iter() .map(flow_timeline_to_bus) .collect(); @@ -1130,6 +1138,10 @@ fn handle_bus_requests(requests: &[BusRequest], responses: &mut Vec } BusRequest::GetProtocolFlowTimeline { protocol } => { let data = node_manager::protocol_timeline_data(protocol) + .unwrap_or_else(|err| { + warn!("Unable to build protocol flow timeline: {err}"); + Vec::new() + }) .into_iter() .map(flow_timeline_to_bus) .collect(); diff --git a/src/rust/lqosd/src/node_manager.rs b/src/rust/lqosd/src/node_manager.rs index 9e1ad720..30f04945 100644 --- a/src/rust/lqosd/src/node_manager.rs +++ b/src/rust/lqosd/src/node_manager.rs @@ -2,6 +2,7 @@ mod auth; pub(crate) mod local_api; mod run; mod runtime_onboarding; +mod security_headers; mod shaper_queries_actor; mod static_pages; mod template; diff --git a/src/rust/lqosd/src/node_manager/auth.rs b/src/rust/lqosd/src/node_manager/auth.rs index a2057954..35752a45 100644 --- a/src/rust/lqosd/src/node_manager/auth.rs +++ b/src/rust/lqosd/src/node_manager/auth.rs @@ -1,7 +1,9 @@ //! Provides authentication for the Node Manager. use crate::node_manager::runtime_onboarding::runtime_onboarding_state; +use crate::node_manager::security_headers::apply_node_manager_security_headers; use axum::Json; +use axum::extract::ConnectInfo; use axum::http::StatusCode; use axum::response::{Html, IntoResponse, Redirect, Response}; use axum_extra::extract::CookieJar; @@ -15,18 +17,24 @@ use parking_lot::Mutex; use rand::RngCore; use serde::{Deserialize, Serialize}; use sha2::Sha256; +use std::collections::{HashMap, VecDeque}; use std::fs::OpenOptions; use std::io::Write; +use std::net::{IpAddr, SocketAddr}; use std::path::{Path, PathBuf}; use std::sync::atomic::AtomicU64; use std::sync::atomic::Ordering::Relaxed; -use std::time::{SystemTime, UNIX_EPOCH}; +use std::time::{Duration, Instant, SystemTime, UNIX_EPOCH}; use tracing::{error, warn}; const COOKIE_NAME: &str = "User-Token"; const SESSION_TOKEN_VERSION: &str = "v1"; const SESSION_DURATION_SECS: u64 = 60 * 60 * 24 * 30; const SESSION_KEY_FILE_NAME: &str = "lqusers.session.key"; +const LOGIN_FAILURE_WINDOW: Duration = Duration::from_secs(60); +const LOGIN_FAILURE_LIMIT: usize = 5; +const LOGIN_REPEATED_FAILURE_LOG_THRESHOLD: usize = 3; +const LOGIN_RATE_LIMIT_USERNAME_MAX_CHARS: usize = 128; type HmacSha256 = Hmac; @@ -48,7 +56,6 @@ struct CachedAuthSnapshot { struct AuthSnapshot { bootstrap_state: AuthBootstrapState, auth_epoch: u64, - allow_anonymous: bool, } #[derive(Clone, Copy, Debug, PartialEq, Eq)] @@ -85,8 +92,174 @@ struct SessionUser { static AUTH_SNAPSHOT: Lazy>> = Lazy::new(|| Mutex::new(None)); static SESSION_KEY: Lazy>>> = Lazy::new(|| Mutex::new(None)); +static LOGIN_RATE_LIMITER: Lazy> = + Lazy::new(|| Mutex::new(LoginRateLimiter::new())); pub static FIRST_LOAD: AtomicU64 = AtomicU64::new(0); +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +enum LoginRateLimitScope { + Ip, + Username, +} + +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +struct LoginRateLimitExceeded { + scope: LoginRateLimitScope, + failures: usize, +} + +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +struct LoginFailureRecord { + ip_failures: usize, + username_failures: usize, +} + +#[derive(Debug)] +struct LoginRateLimiter { + by_ip: HashMap>, + by_username: HashMap>, + last_cleanup: Instant, +} + +impl LoginRateLimiter { + fn new() -> Self { + let now = Instant::now(); + Self { + by_ip: HashMap::new(), + by_username: HashMap::new(), + last_cleanup: now, + } + } + + fn check( + &mut self, + remote_ip: IpAddr, + username: &str, + now: Instant, + ) -> Option { + self.prune_expired_entries(now); + let ip_failures = Self::recent_failures(&mut self.by_ip, &remote_ip, now); + if ip_failures >= LOGIN_FAILURE_LIMIT { + return Some(LoginRateLimitExceeded { + scope: LoginRateLimitScope::Ip, + failures: ip_failures, + }); + } + + let username_key = username.to_string(); + let username_failures = Self::recent_failures(&mut self.by_username, &username_key, now); + if username_failures >= LOGIN_FAILURE_LIMIT { + return Some(LoginRateLimitExceeded { + scope: LoginRateLimitScope::Username, + failures: username_failures, + }); + } + + None + } + + fn record_failure( + &mut self, + remote_ip: IpAddr, + username: &str, + now: Instant, + ) -> LoginFailureRecord { + self.prune_expired_entries(now); + let ip_failures = Self::push_failure(&mut self.by_ip, remote_ip, now); + let username_failures = + Self::push_failure(&mut self.by_username, username.to_string(), now); + + LoginFailureRecord { + ip_failures, + username_failures, + } + } + + fn clear_username(&mut self, username: &str) { + self.by_username.remove(username); + } + + fn prune_expired_entries(&mut self, now: Instant) { + if now.saturating_duration_since(self.last_cleanup) < LOGIN_FAILURE_WINDOW { + return; + } + + Self::prune_map(&mut self.by_ip, now); + Self::prune_map(&mut self.by_username, now); + self.last_cleanup = now; + } + + fn prune_map( + attempts_by_key: &mut HashMap>, + now: Instant, + ) { + attempts_by_key.retain(|_, attempts| { + Self::prune_attempts(attempts, now); + !attempts.is_empty() + }); + } + + fn recent_failures( + attempts_by_key: &mut HashMap>, + key: &K, + now: Instant, + ) -> usize { + let Some(attempts) = attempts_by_key.get_mut(key) else { + return 0; + }; + Self::prune_attempts(attempts, now); + attempts.len() + } + + fn push_failure( + attempts_by_key: &mut HashMap>, + key: K, + now: Instant, + ) -> usize { + let attempts = attempts_by_key.entry(key).or_default(); + Self::prune_attempts(attempts, now); + attempts.push_back(now); + attempts.len() + } + + fn prune_attempts(attempts: &mut VecDeque, now: Instant) { + while let Some(attempt) = attempts.front() { + if now.saturating_duration_since(*attempt) <= LOGIN_FAILURE_WINDOW { + break; + } + attempts.pop_front(); + } + } +} + +fn login_rate_limit_username(username: &str) -> String { + let normalized: String = username + .trim() + .chars() + .flat_map(char::to_lowercase) + .take(LOGIN_RATE_LIMIT_USERNAME_MAX_CHARS) + .collect(); + + if normalized.is_empty() { + "".to_string() + } else { + normalized + } +} + +fn login_rate_limit_response() -> (StatusCode, Json) { + ( + StatusCode::TOO_MANY_REQUESTS, + Json(LoginResponse { + ok: false, + reason: Some("rate_limited"), + message: Some( + "Too many failed login attempts. Wait a minute and try again.".to_string(), + ), + }), + ) +} + fn record_first_login_timestamp_if_needed() { let config = match load_config() { Ok(cfg) => cfg, @@ -161,7 +334,6 @@ fn auth_snapshot() -> AuthSnapshot { return AuthSnapshot { bootstrap_state: AuthBootstrapState::CorruptUsersFile, auth_epoch: 0, - allow_anonymous: false, }; } }; @@ -177,7 +349,6 @@ fn auth_snapshot() -> AuthSnapshot { None => AuthSnapshot { bootstrap_state: AuthBootstrapState::MissingUsersFile, auth_epoch: 0, - allow_anonymous: false, }, Some(_) => match WebUsers::load_or_create() { Ok(users) => AuthSnapshot { @@ -187,14 +358,12 @@ fn auth_snapshot() -> AuthSnapshot { AuthBootstrapState::Ready }, auth_epoch: users.auth_epoch(), - allow_anonymous: users.do_we_allow_anonymous(), }, Err(e) => { warn!("Unable to load auth state: {e}"); AuthSnapshot { bootstrap_state: AuthBootstrapState::CorruptUsersFile, auth_epoch: 0, - allow_anonymous: false, } } }, @@ -264,12 +433,25 @@ fn session_key() -> Result, std::io::Error> { } fn build_session_cookie(token: String) -> Cookie<'static> { + build_session_cookie_with_secure(token, session_cookie_secure()) +} + +fn build_session_cookie_with_secure(token: String, secure: bool) -> Cookie<'static> { let mut cookie = Cookie::new(COOKIE_NAME, token); cookie.set_path("/"); cookie.set_same_site(SameSite::Lax); + cookie.set_http_only(true); + cookie.set_secure(secure); cookie } +fn session_cookie_secure() -> bool { + load_config() + .ok() + .and_then(|config| config.ssl.as_ref().map(|ssl| ssl.enabled)) + .unwrap_or(false) +} + fn build_signed_session(key: &[u8], user: &AuthenticatedUser) -> Result { let now = now_unix_secs(); let claims = SessionClaims { @@ -370,7 +552,9 @@ fn serve_standalone_page(page: &str) -> Result { error!("Unable to read standalone page {:?}: {e}", path); StatusCode::INTERNAL_SERVER_ERROR })?; - Ok(Html(body).into_response()) + let mut response = Html(body).into_response(); + apply_node_manager_security_headers(response.headers_mut()); + Ok(response) } pub async fn login_page(jar: CookieJar) -> Response { @@ -411,14 +595,14 @@ pub async fn get_username(jar: &CookieJar) -> String { } } -#[derive(Copy, Clone, Eq, PartialEq)] +#[derive(Copy, Clone, Debug, Eq, PartialEq)] pub enum LoginResult { Admin, ReadOnly, Denied, } -fn login_result_for_session(user: Option, allow_anonymous: bool) -> LoginResult { +fn login_result_for_session(user: Option) -> LoginResult { match user { Some(SessionUser { role: UserRole::Admin, @@ -428,7 +612,6 @@ fn login_result_for_session(user: Option, allow_anonymous: bool) -> role: UserRole::ReadOnly, .. }) => LoginResult::ReadOnly, - None if allow_anonymous => LoginResult::ReadOnly, None => LoginResult::Denied, } } @@ -463,7 +646,7 @@ fn runtime_onboarding_exempt_path(path: &str) -> bool { /// Checks an incoming request for a `User-Token` cookie. If found, /// it validates the request against the signed session and current auth epoch. /// Missing or empty auth state redirects to first-run; invalid sessions redirect -/// to login unless anonymous read-only is enabled. +/// to login. pub async fn auth_layer( jar: CookieJar, mut req: axum::extract::Request, @@ -481,7 +664,7 @@ pub async fn auth_layer( } let login_result = match session_from_cookie(&jar, &snapshot) { - Ok(user) => login_result_for_session(user, snapshot.allow_anonymous), + Ok(user) => login_result_for_session(user), Err(status) => return (status, "Unable to validate session").into_response(), }; @@ -517,7 +700,7 @@ pub async fn login_from_token(token: &str) -> LoginResult { }; let login_result = match verify_signed_session(&key, token, &snapshot) { - Ok(user) => login_result_for_session(user, snapshot.allow_anonymous), + Ok(user) => login_result_for_session(user), Err(e) => { warn!("Unable to verify websocket session token: {e}"); LoginResult::Denied @@ -531,6 +714,26 @@ pub async fn login_from_token(token: &str) -> LoginResult { login_result } +/// Validates the `User-Token` value from an HTTP Cookie header for websocket upgrades. +pub async fn login_from_cookie_header(cookie_header: Option<&str>) -> LoginResult { + let Some(token) = session_token_from_cookie_header(cookie_header) else { + return LoginResult::Denied; + }; + login_from_token(token).await +} + +fn session_token_from_cookie_header(cookie_header: Option<&str>) -> Option<&str> { + let header = cookie_header?; + header.split(';').find_map(|entry| { + let (name, value) = entry.trim().split_once('=')?; + if name == COOKIE_NAME { + Some(value) + } else { + None + } + }) +} + /// Invalidate the cached auth snapshot after user-management changes. pub fn invalidate_auth_cache() { let mut lock = AUTH_SNAPSHOT.lock(); @@ -549,6 +752,7 @@ pub struct LoginAttempt { } pub async fn try_login( + ConnectInfo(remote_addr): ConnectInfo, jar: CookieJar, Json(login): Json, ) -> Result<(CookieJar, Json), (StatusCode, Json)> { @@ -577,6 +781,23 @@ pub async fn try_login( AuthBootstrapState::Ready => {} } + let remote_ip = remote_addr.ip(); + let rate_limit_username = login_rate_limit_username(&login.username); + let now = Instant::now(); + if let Some(limit) = LOGIN_RATE_LIMITER + .lock() + .check(remote_ip, &rate_limit_username, now) + { + warn!( + remote_ip = %remote_ip, + username = %rate_limit_username, + scope = ?limit.scope, + failures = limit.failures, + "Rate-limited WebUI login attempt" + ); + return Err(login_rate_limit_response()); + } + let mut users = WebUsers::load_or_create().map_err(|e| { warn!("Unable to load users during login: {e}"); ( @@ -591,6 +812,21 @@ pub async fn try_login( let authenticated = users .authenticate(&login.username, &login.password) .map_err(|_| { + let record = + LOGIN_RATE_LIMITER + .lock() + .record_failure(remote_ip, &rate_limit_username, now); + if record.ip_failures >= LOGIN_REPEATED_FAILURE_LOG_THRESHOLD + || record.username_failures >= LOGIN_REPEATED_FAILURE_LOG_THRESHOLD + { + warn!( + remote_ip = %remote_ip, + username = %rate_limit_username, + ip_failures = record.ip_failures, + username_failures = record.username_failures, + "Repeated failed WebUI login attempt" + ); + } ( StatusCode::UNAUTHORIZED, Json(LoginResponse { @@ -601,6 +837,9 @@ pub async fn try_login( ) })?; + LOGIN_RATE_LIMITER + .lock() + .clear_username(&rate_limit_username); invalidate_auth_cache(); let key = session_key().map_err(|e| { error!("Unable to load session key during login: {e}"); @@ -639,7 +878,6 @@ pub async fn try_login( pub struct FirstUser { username: String, password: String, - allow_anonymous: bool, } pub async fn first_user( @@ -682,19 +920,6 @@ pub async fn first_user( }), ) })?; - users - .allow_anonymous(new_user.allow_anonymous) - .map_err(|e| { - warn!("Unable to set anonymous auth policy: {e}"); - ( - StatusCode::INTERNAL_SERVER_ERROR, - Json(LoginResponse { - ok: false, - reason: Some("auth_corrupt"), - message: Some("Unable to update auth settings.".to_string()), - }), - ) - })?; users .add_or_update_user(&new_user.username, &new_user.password, UserRole::Admin) .map_err(|e| { @@ -748,3 +973,147 @@ pub async fn first_user( }), )) } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn missing_session_is_denied() { + assert_eq!(login_result_for_session(None), LoginResult::Denied); + } + + #[test] + fn authenticated_read_only_session_keeps_read_only_role() { + let user = SessionUser { + username: "support".to_string(), + role: UserRole::ReadOnly, + }; + + assert_eq!(login_result_for_session(Some(user)), LoginResult::ReadOnly); + } + + #[test] + fn login_rate_limiter_blocks_after_failed_attempt_limit() { + let mut limiter = LoginRateLimiter::new(); + let remote_ip = IpAddr::from([203, 0, 113, 10]); + let username = "admin"; + let now = Instant::now(); + + for attempt in 1..=LOGIN_FAILURE_LIMIT { + assert_eq!(limiter.check(remote_ip, username, now), None); + let record = limiter.record_failure(remote_ip, username, now); + assert_eq!(record.ip_failures, attempt); + assert_eq!(record.username_failures, attempt); + } + + assert_eq!( + limiter.check(remote_ip, username, now), + Some(LoginRateLimitExceeded { + scope: LoginRateLimitScope::Ip, + failures: LOGIN_FAILURE_LIMIT + }) + ); + } + + #[test] + fn login_rate_limiter_blocks_username_across_source_ips() { + let mut limiter = LoginRateLimiter::new(); + let username = "admin"; + let now = Instant::now(); + + for host in 1..=LOGIN_FAILURE_LIMIT { + let remote_ip = IpAddr::from([198, 51, 100, host as u8]); + assert_eq!(limiter.check(remote_ip, username, now), None); + limiter.record_failure(remote_ip, username, now); + } + + let fresh_remote_ip = IpAddr::from([198, 51, 100, 200]); + assert_eq!( + limiter.check(fresh_remote_ip, username, now), + Some(LoginRateLimitExceeded { + scope: LoginRateLimitScope::Username, + failures: LOGIN_FAILURE_LIMIT + }) + ); + } + + #[test] + fn login_rate_limiter_expires_old_failures_and_clears_on_success() { + let mut limiter = LoginRateLimiter::new(); + let remote_ip = IpAddr::from([192, 0, 2, 44]); + let username = "admin"; + let now = Instant::now(); + + for _ in 0..LOGIN_FAILURE_LIMIT { + limiter.record_failure(remote_ip, username, now); + } + assert!(limiter.check(remote_ip, username, now).is_some()); + + let later = now + LOGIN_FAILURE_WINDOW + Duration::from_secs(1); + assert_eq!(limiter.check(remote_ip, username, later), None); + assert!(limiter.by_ip.is_empty()); + assert!(limiter.by_username.is_empty()); + + for _ in 0..LOGIN_FAILURE_LIMIT { + limiter.record_failure(remote_ip, username, later); + } + assert!(limiter.check(remote_ip, username, later).is_some()); + + limiter.clear_username(username); + assert_eq!( + limiter.check(remote_ip, username, later), + Some(LoginRateLimitExceeded { + scope: LoginRateLimitScope::Ip, + failures: LOGIN_FAILURE_LIMIT + }) + ); + + let later_after_window = later + LOGIN_FAILURE_WINDOW + Duration::from_secs(1); + assert_eq!(limiter.check(remote_ip, username, later_after_window), None); + } + + #[test] + fn login_rate_limit_username_normalizes_and_caps_input() { + assert_eq!(login_rate_limit_username(" Admin "), "admin"); + assert_eq!(login_rate_limit_username(" "), ""); + + let long_name = "A".repeat(LOGIN_RATE_LIMIT_USERNAME_MAX_CHARS + 10); + assert_eq!( + login_rate_limit_username(&long_name).chars().count(), + LOGIN_RATE_LIMIT_USERNAME_MAX_CHARS + ); + } + + #[test] + fn session_cookie_is_http_only_and_secure_when_requested() { + let cookie = build_session_cookie_with_secure("session-value".to_string(), true); + let rendered = cookie.to_string(); + + assert!(rendered.contains("HttpOnly")); + assert!(rendered.contains("SameSite=Lax")); + assert!(rendered.contains("Secure")); + } + + #[test] + fn session_cookie_omits_secure_for_direct_http_mode() { + let cookie = build_session_cookie_with_secure("session-value".to_string(), false); + let rendered = cookie.to_string(); + + assert!(rendered.contains("HttpOnly")); + assert!(rendered.contains("SameSite=Lax")); + assert!(!rendered.contains("Secure")); + } + + #[test] + fn session_cookie_header_parser_extracts_user_token() { + assert_eq!( + session_token_from_cookie_header(Some( + "Theme=dark; User-Token=v1.payload.sig; other=1" + )), + Some("v1.payload.sig") + ); + assert_eq!(session_token_from_cookie_header(Some("Theme=dark")), None); + assert_eq!(session_token_from_cookie_header(None), None); + } +} diff --git a/src/rust/lqosd/src/node_manager/js_build/src/circuit.js b/src/rust/lqosd/src/node_manager/js_build/src/circuit.js index 716a44b8..3334f7c1 100644 --- a/src/rust/lqosd/src/node_manager/js_build/src/circuit.js +++ b/src/rust/lqosd/src/node_manager/js_build/src/circuit.js @@ -1,6 +1,6 @@ // Obtain URL parameters import {DirectChannel} from "./pubsub/direct_channels"; -import {clearDiv, formatLastSeen, simpleRow, simpleRowHtml, theading} from "./helpers/builders"; +import {clearDiv, formatLastSeen, simpleRow, simpleRowTrustedHtml, theading} from "./helpers/builders"; import { formatRetransmitFraction, formatRtt, @@ -29,6 +29,11 @@ import {CakeDrops} from "./graphs/cake_drops"; import {QueuingActivityWaveform} from "./graphs/queuing_activity_waveform"; import {getNodeIdMap, linkToTreeNode} from "./executive_utils"; import {loadConfig} from "./config/config_helper"; +import { + appendRedactableText, + setIconText, + setPacketCaptureDownloadButton, +} from "./circuit_packet_capture_dom.mjs"; const params = new Proxy(new URLSearchParams(window.location.search), { get: (searchParams, prop) => searchParams.get(prop), @@ -58,6 +63,7 @@ let excludeRttLastValue = false; let excludeRttBusy = false; let latestCakeMsg = null; let cakeGraphs = null; + let cakeQueueUnavailable = false; let queueStatsMode = "live"; let circuitSqmOverride = ""; @@ -1914,14 +1920,14 @@ function renderTopAsnTab() { row.appendChild(truncatedTrafficCell(rowData.asn_name, "lqos-circuit-traffic-asn-cell")); row.appendChild(truncatedTrafficCell(rowData.asn_country, "lqos-circuit-traffic-country-cell")); - row.appendChild(simpleRowHtml(formatThroughput(rowData.down_bps, plan.down))); - row.appendChild(simpleRowHtml(formatThroughput(rowData.up_bps, plan.up))); - row.appendChild(simpleRowHtml(formatRttNanos(rowData.rtt_down_nanos))); - row.appendChild(simpleRowHtml(formatRttNanos(rowData.rtt_up_nanos))); - row.appendChild(simpleRowHtml(formatQooScore(rowData.qoo_down))); - row.appendChild(simpleRowHtml(formatQooScore(rowData.qoo_up))); - row.appendChild(simpleRowHtml(rowData.retransmit_down_pct > 0 ? formatRetransmitFraction(rowData.retransmit_down_pct) : "-")); - row.appendChild(simpleRowHtml(rowData.retransmit_up_pct > 0 ? formatRetransmitFraction(rowData.retransmit_up_pct) : "-")); + row.appendChild(simpleRowTrustedHtml(formatThroughput(rowData.down_bps, plan.down))); + row.appendChild(simpleRowTrustedHtml(formatThroughput(rowData.up_bps, plan.up))); + row.appendChild(simpleRowTrustedHtml(formatRttNanos(rowData.rtt_down_nanos))); + row.appendChild(simpleRowTrustedHtml(formatRttNanos(rowData.rtt_up_nanos))); + row.appendChild(simpleRowTrustedHtml(formatQooScore(rowData.qoo_down))); + row.appendChild(simpleRowTrustedHtml(formatQooScore(rowData.qoo_up))); + row.appendChild(simpleRowTrustedHtml(rowData.retransmit_down_pct > 0 ? formatRetransmitFraction(rowData.retransmit_down_pct) : "-")); + row.appendChild(simpleRowTrustedHtml(rowData.retransmit_up_pct > 0 ? formatRetransmitFraction(rowData.retransmit_up_pct) : "-")); row.appendChild(simpleRow(scaleNumber(rowData.flow_count))); tbody.appendChild(row); @@ -2003,18 +2009,18 @@ function renderTrafficTab() { row.style.opacity = toNumber(rowData.opacity, 1); row.appendChild(truncatedTrafficCell(rowData.protocol_name, "lqos-circuit-traffic-protocol-cell")); - row.appendChild(simpleRowHtml(formatThroughput(rowData.down_bps, plan.down))); - row.appendChild(simpleRowHtml(formatThroughput(rowData.up_bps, plan.up))); + row.appendChild(simpleRowTrustedHtml(formatThroughput(rowData.down_bps, plan.down))); + row.appendChild(simpleRowTrustedHtml(formatThroughput(rowData.up_bps, plan.up))); row.appendChild(simpleRow(scaleNumber(rowData.bytes_sent_down))); row.appendChild(simpleRow(scaleNumber(rowData.bytes_sent_up))); row.appendChild(simpleRow(scaleNumber(rowData.packets_sent_down))); row.appendChild(simpleRow(scaleNumber(rowData.packets_sent_up))); - row.appendChild(simpleRowHtml(rowData.retransmit_down_pct > 0 ? formatRetransmitFraction(rowData.retransmit_down_pct) : "-")); - row.appendChild(simpleRowHtml(rowData.retransmit_up_pct > 0 ? formatRetransmitFraction(rowData.retransmit_up_pct) : "-")); - row.appendChild(simpleRowHtml(formatRttNanos(rowData.rtt_down_nanos))); - row.appendChild(simpleRowHtml(formatRttNanos(rowData.rtt_up_nanos))); - row.appendChild(simpleRowHtml(formatQooScore(rowData.qoo_down))); - row.appendChild(simpleRowHtml(formatQooScore(rowData.qoo_up))); + row.appendChild(simpleRowTrustedHtml(rowData.retransmit_down_pct > 0 ? formatRetransmitFraction(rowData.retransmit_down_pct) : "-")); + row.appendChild(simpleRowTrustedHtml(rowData.retransmit_up_pct > 0 ? formatRetransmitFraction(rowData.retransmit_up_pct) : "-")); + row.appendChild(simpleRowTrustedHtml(formatRttNanos(rowData.rtt_down_nanos))); + row.appendChild(simpleRowTrustedHtml(formatRttNanos(rowData.rtt_up_nanos))); + row.appendChild(simpleRowTrustedHtml(formatQooScore(rowData.qoo_down))); + row.appendChild(simpleRowTrustedHtml(formatQooScore(rowData.qoo_up))); row.appendChild(truncatedTrafficCell(rowData.asn_name, "lqos-circuit-traffic-asn-cell")); row.appendChild(truncatedTrafficCell(rowData.asn_country, "lqos-circuit-traffic-country-cell")); row.appendChild(simpleRow(rowData.remote_ip)); @@ -2161,7 +2167,10 @@ function initialDevices(circuits) { let name = document.createElement("h5"); name.classList.add("redactable"); - name.innerHTML = " " + circuit.device_name; + const deviceIcon = document.createElement("i"); + deviceIcon.classList.add("fa", "fa-computer"); + name.appendChild(deviceIcon); + name.appendChild(document.createTextNode(` ${circuit.device_name ?? ""}`)); d.appendChild(name); let infoTableWrap = document.createElement("div"); @@ -2181,7 +2190,7 @@ function initialDevices(circuits) { td.classList.add("table-value-cell"); td.classList.add("redactable"); td.colSpan = 2; - td.innerHTML = circuit.mac; + td.textContent = circuit.mac ?? ""; tr.appendChild(td); tbody.appendChild(tr); @@ -2194,7 +2203,7 @@ function initialDevices(circuits) { td = document.createElement("td"); td.classList.add("table-value-cell"); td.colSpan = 2; - td.innerHTML = circuit.comment; + td.textContent = circuit.comment ?? ""; tr2.appendChild(td); tbody.appendChild(tr2); @@ -2498,7 +2507,7 @@ function wireupAnalysis(circuits) { listBtn.id = "CaptureTopBtn"; listBtn.classList.add("btn", "btn-secondary", "dropdown-toggle", "btn-sm"); listBtn.setAttribute("data-bs-toggle", "dropdown"); - listBtn.innerHTML = " Packet Capture"; + setIconText(listBtn, ["fa", "fa-search"], "Packet Capture"); list.appendChild(listBtn); let listUl = document.createElement("ul"); @@ -2507,8 +2516,9 @@ function wireupAnalysis(circuits) { let entry = document.createElement("li"); let item = document.createElement("a"); item.classList.add("dropdown-item"); - item.innerHTML = " Capture packets from " + ip[0] + ""; let address = ip[0]; // For closure capture + setIconText(item, ["fa", "fa-search"], "Capture packets from "); + appendRedactableText(item, address); item.onclick = () => { //console.log("Clicky " + address); listenOnce("RequestAnalysisResult", (msg) => { @@ -2522,13 +2532,13 @@ function wireupAnalysis(circuits) { let sessionId = okData.session_id; let btn = document.getElementById("CaptureTopBtn"); btn.disabled = true; - btn.innerHTML = " Capturing Packets (" + counter + ")"; + setIconText(btn, ["fa", "fa-spinner", "fa-spin"], "Capturing Packets (" + counter + ")"); let interval = setInterval(() => { counter--; if (counter === -1) { clearInterval(interval); btn.disabled = false; - btn.innerHTML = " Download Packet Capture for " + address + ""; + setPacketCaptureDownloadButton(btn, address); btn.classList.remove("btn-secondary"); btn.classList.add("btn-success"); btn.onclick = () => { @@ -2543,7 +2553,7 @@ function wireupAnalysis(circuits) { } return; } - btn.innerHTML = " Capturing Packets (" + counter + ")"; + setIconText(btn, ["fa", "fa-spinner", "fa-spin"], "Capturing Packets (" + counter + ")"); }, 1000); }); wsClient.send({ RequestAnalysis: { ip: address } }); diff --git a/src/rust/lqosd/src/node_manager/js_build/src/circuit_packet_capture_dom.mjs b/src/rust/lqosd/src/node_manager/js_build/src/circuit_packet_capture_dom.mjs new file mode 100644 index 00000000..b8ab403a --- /dev/null +++ b/src/rust/lqosd/src/node_manager/js_build/src/circuit_packet_capture_dom.mjs @@ -0,0 +1,20 @@ +import {appendIconText} from "./helpers/safe_dom.mjs"; + +export function setIconText(target, iconClasses, text) { + while (target.firstChild) { + target.removeChild(target.firstChild); + } + appendIconText(target, iconClasses, text); +} + +export function appendRedactableText(target, text) { + const span = document.createElement("span"); + span.classList.add("redactable"); + span.textContent = text ?? ""; + target.appendChild(span); +} + +export function setPacketCaptureDownloadButton(button, address) { + setIconText(button, ["fa", "fa-download"], "Download Packet Capture for "); + appendRedactableText(button, address); +} diff --git a/src/rust/lqosd/src/node_manager/js_build/src/circuit_packet_capture_dom.test.mjs b/src/rust/lqosd/src/node_manager/js_build/src/circuit_packet_capture_dom.test.mjs new file mode 100644 index 00000000..4dc530c3 --- /dev/null +++ b/src/rust/lqosd/src/node_manager/js_build/src/circuit_packet_capture_dom.test.mjs @@ -0,0 +1,111 @@ +import assert from "node:assert/strict"; +import test from "node:test"; + +import { + appendRedactableText, + setIconText, + setPacketCaptureDownloadButton, +} from "./circuit_packet_capture_dom.mjs"; + +class FakeClassList { + constructor() { + this.values = []; + } + + add(...classes) { + this.values.push(...classes); + } + + contains(className) { + return this.values.includes(className); + } +} + +class FakeElement { + constructor(tagName) { + this.tagName = tagName.toUpperCase(); + this.children = []; + this.classList = new FakeClassList(); + this.attributes = {}; + this._textContent = ""; + } + + appendChild(child) { + this.children.push(child); + return child; + } + + removeChild(child) { + const index = this.children.indexOf(child); + if (index >= 0) { + this.children.splice(index, 1); + } + return child; + } + + get firstChild() { + return this.children[0] ?? null; + } + + setAttribute(name, value) { + this.attributes[name] = String(value); + } + + set textContent(value) { + this._textContent = String(value); + } + + get textContent() { + return this._textContent; + } + + set innerHTML(_) { + throw new Error("packet-capture helpers must not use innerHTML"); + } +} + +class FakeTextNode { + constructor(text) { + this.textContent = String(text); + } +} + +function installFakeDocument() { + globalThis.document = { + createElement: (tagName) => new FakeElement(tagName), + createTextNode: (textValue) => new FakeTextNode(textValue), + }; +} + +test("packet-capture menu text is rendered as DOM text", () => { + installFakeDocument(); + const link = new FakeElement("a"); + const address = ""; + + setIconText(link, ["fa", "fa-search"], "Capture packets from "); + appendRedactableText(link, address); + + assert.equal(link.children.length, 4); + assert.equal(link.children[0].tagName, "I"); + assert.equal(link.children[0].attributes["aria-hidden"], "true"); + assert.deepEqual(link.children[0].classList.values, ["fa", "fa-search"]); + assert.equal(link.children[2].textContent, "Capture packets from "); + assert.equal(link.children[3].tagName, "SPAN"); + assert.equal(link.children[3].textContent, address); + assert.equal(link.children[3].classList.contains("redactable"), true); +}); + +test("packet-capture download button resets children without innerHTML", () => { + installFakeDocument(); + const button = new FakeElement("button"); + const address = ""; + button.appendChild(new FakeTextNode("old label")); + + setPacketCaptureDownloadButton(button, address); + + assert.equal(button.children.length, 4); + assert.equal(button.children[0].tagName, "I"); + assert.deepEqual(button.children[0].classList.values, ["fa", "fa-download"]); + assert.equal(button.children[2].textContent, "Download Packet Capture for "); + assert.equal(button.children[3].textContent, address); +}); diff --git a/src/rust/lqosd/src/node_manager/js_build/src/config/network_mode_storage.mjs b/src/rust/lqosd/src/node_manager/js_build/src/config/network_mode_storage.mjs new file mode 100644 index 00000000..ce9eb8c7 --- /dev/null +++ b/src/rust/lqosd/src/node_manager/js_build/src/config/network_mode_storage.mjs @@ -0,0 +1,73 @@ +export const DRAFT_KEY = "lqos-network-mode-draft"; +export const PENDING_OPERATION_KEY = "lqos-network-mode-pending"; + +const OPERATIONAL_NETWORK_MODE_KEYS = [ + DRAFT_KEY, + PENDING_OPERATION_KEY, +]; + +function storageOf(kind) { + try { + return globalThis.window?.[kind] ?? globalThis[kind] ?? null; + } catch (_) { + return null; + } +} + +function readJson(storage, key) { + if (!storage) return null; + try { + const raw = storage.getItem(key); + if (!raw) return null; + return JSON.parse(raw); + } catch (_) { + return null; + } +} + +export function loadNetworkModeState(key) { + const session = storageOf("sessionStorage"); + const value = readJson(session, key); + if (value !== null) return value; + + const local = storageOf("localStorage"); + const legacyValue = readJson(local, key); + if (legacyValue !== null && session) { + try { + session.setItem(key, JSON.stringify(legacyValue)); + } catch (_) {} + } + if (local) { + try { + local.removeItem(key); + } catch (_) {} + } + return legacyValue; +} + +export function saveNetworkModeState(key, value) { + const session = storageOf("sessionStorage"); + if (session) { + session.setItem(key, JSON.stringify(value)); + } + removeLegacyNetworkModeState(key); +} + +export function clearNetworkModeState(key) { + const session = storageOf("sessionStorage"); + if (session) { + session.removeItem(key); + } + removeLegacyNetworkModeState(key); +} + +export function clearOperationalNetworkModeStorage() { + OPERATIONAL_NETWORK_MODE_KEYS.forEach(clearNetworkModeState); +} + +function removeLegacyNetworkModeState(key) { + const local = storageOf("localStorage"); + if (local) { + local.removeItem(key); + } +} diff --git a/src/rust/lqosd/src/node_manager/js_build/src/config/network_mode_storage.test.mjs b/src/rust/lqosd/src/node_manager/js_build/src/config/network_mode_storage.test.mjs new file mode 100644 index 00000000..45929494 --- /dev/null +++ b/src/rust/lqosd/src/node_manager/js_build/src/config/network_mode_storage.test.mjs @@ -0,0 +1,103 @@ +import assert from "node:assert/strict"; +import test from "node:test"; + +import { + clearOperationalNetworkModeStorage, + DRAFT_KEY, + loadNetworkModeState, + PENDING_OPERATION_KEY, + saveNetworkModeState, +} from "./network_mode_storage.mjs"; + +class FakeStorage { + constructor(entries = {}) { + this.values = new Map(Object.entries(entries)); + } + + getItem(key) { + return this.values.has(key) ? this.values.get(key) : null; + } + + setItem(key, value) { + this.values.set(key, String(value)); + } + + removeItem(key) { + this.values.delete(key); + } +} + +function withStorage({localEntries = {}, sessionEntries = {}}, callback) { + const previousWindow = globalThis.window; + const localStorage = new FakeStorage(localEntries); + const sessionStorage = new FakeStorage(sessionEntries); + + globalThis.window = {localStorage, sessionStorage}; + try { + callback({localStorage, sessionStorage}); + } finally { + if (previousWindow === undefined) { + delete globalThis.window; + } else { + globalThis.window = previousWindow; + } + } +} + +test("saves network mode draft state only in tab storage", () => { + withStorage({ + localEntries: { + [DRAFT_KEY]: JSON.stringify({mode: "legacy"}), + }, + }, ({localStorage, sessionStorage}) => { + saveNetworkModeState(DRAFT_KEY, {mode: "bridge"}); + + assert.deepEqual(loadNetworkModeState(DRAFT_KEY), {mode: "bridge"}); + assert.equal(sessionStorage.getItem(DRAFT_KEY), JSON.stringify({mode: "bridge"})); + assert.equal(localStorage.getItem(DRAFT_KEY), null); + }); +}); + +test("migrates existing localStorage drafts into tab storage once", () => { + withStorage({ + localEntries: { + [DRAFT_KEY]: JSON.stringify({mode: "single_interface", interface: "eth0"}), + }, + }, ({localStorage, sessionStorage}) => { + assert.deepEqual(loadNetworkModeState(DRAFT_KEY), {mode: "single_interface", interface: "eth0"}); + assert.equal(sessionStorage.getItem(DRAFT_KEY), JSON.stringify({mode: "single_interface", interface: "eth0"})); + assert.equal(localStorage.getItem(DRAFT_KEY), null); + }); +}); + +test("drops malformed legacy network mode state", () => { + withStorage({ + localEntries: { + [PENDING_OPERATION_KEY]: "{not-json", + }, + }, ({localStorage, sessionStorage}) => { + assert.equal(loadNetworkModeState(PENDING_OPERATION_KEY), null); + assert.equal(sessionStorage.getItem(PENDING_OPERATION_KEY), null); + assert.equal(localStorage.getItem(PENDING_OPERATION_KEY), null); + }); +}); + +test("clears operational network mode state without touching durable preferences", () => { + withStorage({ + localEntries: { + [DRAFT_KEY]: JSON.stringify({mode: "legacy"}), + "lqos-theme": "dark", + }, + sessionEntries: { + [DRAFT_KEY]: JSON.stringify({mode: "bridge"}), + [PENDING_OPERATION_KEY]: JSON.stringify({operation_id: "op-1"}), + }, + }, ({localStorage, sessionStorage}) => { + clearOperationalNetworkModeStorage(); + + assert.equal(sessionStorage.getItem(DRAFT_KEY), null); + assert.equal(sessionStorage.getItem(PENDING_OPERATION_KEY), null); + assert.equal(localStorage.getItem(DRAFT_KEY), null); + assert.equal(localStorage.getItem("lqos-theme"), "dark"); + }); +}); diff --git a/src/rust/lqosd/src/node_manager/js_build/src/config_interface.js b/src/rust/lqosd/src/node_manager/js_build/src/config_interface.js index 696b1d3e..833859c6 100644 --- a/src/rust/lqosd/src/node_manager/js_build/src/config_interface.js +++ b/src/rust/lqosd/src/node_manager/js_build/src/config_interface.js @@ -1,8 +1,13 @@ import {loadConfig, renderConfigMenu} from "./config/config_helper"; import {hydrateDraftMtu, MTU_MAX, MTU_MIN, parseOptionalMtu} from "./config/network_mode_mtu.mjs"; +import { + clearNetworkModeState, + DRAFT_KEY, + loadNetworkModeState, + PENDING_OPERATION_KEY, + saveNetworkModeState, +} from "./config/network_mode_storage.mjs"; -const DRAFT_KEY = "lqos-network-mode-draft"; -const PENDING_OPERATION_KEY = "lqos-network-mode-pending"; const NETPLAN_TRY_TIMEOUT_MS = 30_000; const RECONNECT_POLL_INTERVAL_MS = 2_000; const BRIDGE_MTU_HELP = "Applies to the bridge members and br0."; @@ -364,29 +369,17 @@ function populateFormFromConfig(config) { } function loadDraft() { - try { - const raw = localStorage.getItem(DRAFT_KEY); - if (!raw) return null; - return JSON.parse(raw); - } catch (_) { - return null; - } + return loadNetworkModeState(DRAFT_KEY); } function saveDraft() { const draft = buildCandidateConfig(); - localStorage.setItem(DRAFT_KEY, JSON.stringify(draft)); + saveNetworkModeState(DRAFT_KEY, draft); return draft; } function loadPendingOperation() { - try { - const raw = localStorage.getItem(PENDING_OPERATION_KEY); - if (!raw) return null; - return JSON.parse(raw); - } catch (_) { - return null; - } + return loadNetworkModeState(PENDING_OPERATION_KEY); } function pendingDeadlineMs(operation) { @@ -421,14 +414,14 @@ function setPendingOperation(operation, extra = {}) { ...extra, saved_at_ms: Date.now(), }; - localStorage.setItem(PENDING_OPERATION_KEY, JSON.stringify(currentPendingOperation)); + saveNetworkModeState(PENDING_OPERATION_KEY, currentPendingOperation); renderRecoveryPanel(); ensureRecoveryPolling(); } function clearPendingOperation() { currentPendingOperation = null; - localStorage.removeItem(PENDING_OPERATION_KEY); + clearNetworkModeState(PENDING_OPERATION_KEY); if (reconnectPollTimer) { clearInterval(reconnectPollTimer); reconnectPollTimer = null; @@ -838,7 +831,7 @@ function applyNetworkChanges(mode = "Apply") { action_label: actionLabel, }); } - localStorage.removeItem(DRAFT_KEY); + clearNetworkModeState(DRAFT_KEY); window.config = candidate; renderRecoveryPanel(); window.scrollTo({ top: 0, behavior: "smooth" }); @@ -865,6 +858,7 @@ function actOnPending(endpoint) { postJson(endpoint, { operation_id: operationId }) .then((response) => { clearPendingOperation(); + clearNetworkModeState(DRAFT_KEY); alert(response?.message || "Operation completed"); return Promise.allSettled([refreshHelperStatus(), inspectCandidate()]); }) @@ -896,6 +890,7 @@ function rollbackLatestBackup() { postJson("/local-api/network-mode/rollback", { backup_id: backupId }) .then((response) => { + clearNetworkModeState(DRAFT_KEY); alert(response?.message || "Rollback completed"); return Promise.all([refreshHelperStatus(), inspectCandidate()]); }) @@ -960,7 +955,7 @@ function wireActions() { if (!validateConfig()) return; saveDraft(); inspectCandidate(); - alert("Network mode draft saved in this browser. Use Apply Network Changes to commit both lqos.conf and netplan together."); + alert("Network mode draft saved for this browser tab. Use Apply Network Changes to commit both lqos.conf and netplan together."); }); document.getElementById("inspectButton").addEventListener("click", inspectCandidate); document.getElementById("applyButton").addEventListener("click", () => applyNetworkChanges("Apply")); diff --git a/src/rust/lqosd/src/node_manager/js_build/src/config_users.js b/src/rust/lqosd/src/node_manager/js_build/src/config_users.js index f7cd4fc8..a04c109b 100644 --- a/src/rust/lqosd/src/node_manager/js_build/src/config_users.js +++ b/src/rust/lqosd/src/node_manager/js_build/src/config_users.js @@ -97,17 +97,22 @@ function loadUsers() { const tbody = $(''); users.forEach(user => { + const actions = $(''); + const editButton = $('')) + .append(document.createTextNode(' Edit')); + const deleteButton = $(' - - `); + .append($('').text(user.username)) + .append($('').text(user.role)) + .append(actions); tbody.append(row); }); diff --git a/src/rust/lqosd/src/node_manager/js_build/src/dashlets/circuit_capacity_dash.js b/src/rust/lqosd/src/node_manager/js_build/src/dashlets/circuit_capacity_dash.js index ea33900d..72ffa0d1 100644 --- a/src/rust/lqosd/src/node_manager/js_build/src/dashlets/circuit_capacity_dash.js +++ b/src/rust/lqosd/src/node_manager/js_build/src/dashlets/circuit_capacity_dash.js @@ -1,5 +1,5 @@ import {BaseDashlet} from "../lq_js_common/dashboard/base_dashlet"; -import {clearDashDiv, simpleRowHtml, theading} from "../helpers/builders"; +import {clearDashDiv, simpleRowTrustedHtml, theading} from "../helpers/builders"; import {formatRtt, formatPercent} from "../helpers/scaling"; import {redactCell} from "../helpers/redact"; import {TimedCache} from "../lq_js_common/helpers/timed_cache"; @@ -87,9 +87,9 @@ export class CircuitCapacityDash extends BaseDashlet { linkCol.appendChild(link); row.appendChild(linkCol); - row.appendChild(simpleRowHtml(formatPercent(c.capacity[0]*100))); - row.appendChild(simpleRowHtml(formatPercent(c.capacity[1]*100))); - row.appendChild(simpleRowHtml(formatRtt(c.rtt))); + row.appendChild(simpleRowTrustedHtml(formatPercent(c.capacity[0]*100))); + row.appendChild(simpleRowTrustedHtml(formatPercent(c.capacity[1]*100))); + row.appendChild(simpleRowTrustedHtml(formatRtt(c.rtt))); tbody.appendChild(row); count++; diff --git a/src/rust/lqosd/src/node_manager/js_build/src/dashlets/ltsShaperStatus.js b/src/rust/lqosd/src/node_manager/js_build/src/dashlets/ltsShaperStatus.js index 3b12f4fc..d24b787e 100644 --- a/src/rust/lqosd/src/node_manager/js_build/src/dashlets/ltsShaperStatus.js +++ b/src/rust/lqosd/src/node_manager/js_build/src/dashlets/ltsShaperStatus.js @@ -1,5 +1,5 @@ import {BaseDashlet} from "../lq_js_common/dashboard/base_dashlet"; -import {clearDiv, simpleRow, simpleRowHtml, theading} from "../helpers/builders"; +import {clearDiv, simpleRow, simpleRowTrustedHtml, theading} from "../helpers/builders"; import {get_ws_client} from "../pubsub/ws"; const wsClient = get_ws_client(); @@ -74,7 +74,7 @@ export class LtsShaperStatus extends BaseDashlet { } else if (row.last_seen_seconds_ago > 120) { color = "orange"; } - tr.appendChild(simpleRowHtml(`■`)); + tr.appendChild(simpleRowTrustedHtml(`■`)); tr.appendChild(simpleRow(row.name)); tr.appendChild(simpleRow(row.last_seen_seconds_ago + "s")); tbody.appendChild(tr); diff --git a/src/rust/lqosd/src/node_manager/js_build/src/dashlets/top10_downloaders.js b/src/rust/lqosd/src/node_manager/js_build/src/dashlets/top10_downloaders.js index 52a4f875..c08e162c 100644 --- a/src/rust/lqosd/src/node_manager/js_build/src/dashlets/top10_downloaders.js +++ b/src/rust/lqosd/src/node_manager/js_build/src/dashlets/top10_downloaders.js @@ -1,4 +1,11 @@ -import {clearDashDiv, simpleRow, simpleRowHtml, theading, TopNTableFromMsgData} from "../helpers/builders"; +import { + clearDashDiv, + simpleLinkRow, + simpleRow, + simpleRowTrustedHtml, + theading, + TopNTableFromMsgData, +} from "../helpers/builders"; import {TimedCache} from "../lq_js_common/helpers/timed_cache"; import {periodNameToSeconds} from "../helpers/time_periods"; import {formatRetransmit, formatRtt} from "../helpers/scaling"; @@ -94,15 +101,19 @@ export class Top10Downloaders extends DashletBaseInsight { //console.log(row); let tr = document.createElement("tr"); tr.classList.add("small"); - tr.appendChild(simpleRowHtml("" + row.circuit_name + "")); + tr.appendChild(simpleLinkRow( + `circuit.html?circuit=${encodeURIComponent(row.circuit_hash ?? "")}`, + row.circuit_name, + true, + )); tr.appendChild(simpleRow(scaleNumber(toNumber(row.bytes_down, 0) * 1000000, 0))); if (row.rtt !== null) { - tr.appendChild(simpleRowHtml(formatRtt(row.rtt))); + tr.appendChild(simpleRowTrustedHtml(formatRtt(row.rtt))); } else { tr.appendChild(simpleRow("-")); } if (row.rxmit !== null) { - tr.appendChild(simpleRowHtml(formatRetransmit(row.rxmit))); + tr.appendChild(simpleRowTrustedHtml(formatRetransmit(row.rxmit))); } else { tr.appendChild(simpleRow("-")); } diff --git a/src/rust/lqosd/src/node_manager/js_build/src/dashlets/top10flows_bytes.js b/src/rust/lqosd/src/node_manager/js_build/src/dashlets/top10flows_bytes.js index c5c10d39..3a633f59 100644 --- a/src/rust/lqosd/src/node_manager/js_build/src/dashlets/top10flows_bytes.js +++ b/src/rust/lqosd/src/node_manager/js_build/src/dashlets/top10flows_bytes.js @@ -1,4 +1,4 @@ -import {clearDashDiv, simpleRow, simpleRowHtml, theading} from "../helpers/builders"; +import {clearDashDiv, simpleRow, simpleRowTrustedHtml, theading} from "../helpers/builders"; import {scaleNumber, toNumber} from "../lq_js_common/helpers/scaling"; import {formatRtt} from "../helpers/scaling"; import {TrimToFit} from "../lq_js_common/helpers/text_utils"; @@ -202,27 +202,27 @@ export class Top10FlowsBytes extends DashletBaseInsight { tr.classList.add("small"); tr.appendChild(simpleRow(row.circuit_name || "", true)); tr.appendChild(simpleRow(row.protocol)); - tr.appendChild(simpleRowHtml(scaleNumber(row.bytes_down))); - tr.appendChild(simpleRowHtml(scaleNumber(row.bytes_up))); + tr.appendChild(simpleRowTrustedHtml(scaleNumber(row.bytes_down))); + tr.appendChild(simpleRowTrustedHtml(scaleNumber(row.bytes_up))); if (row.rtt_down === null) { - tr.appendChild(simpleRowHtml("-")); + tr.appendChild(simpleRowTrustedHtml("-")); } else { - tr.appendChild(simpleRowHtml(formatRtt(row.rtt_down))); + tr.appendChild(simpleRowTrustedHtml(formatRtt(row.rtt_down))); } if (row.rtt_up === null) { - tr.appendChild(simpleRowHtml("-")); + tr.appendChild(simpleRowTrustedHtml("-")); } else { - tr.appendChild(simpleRowHtml(formatRtt(row.rtt_up))); + tr.appendChild(simpleRowTrustedHtml(formatRtt(row.rtt_up))); } if (row.rxmit_down === null) { - tr.appendChild(simpleRowHtml("-")); + tr.appendChild(simpleRowTrustedHtml("-")); } else { - tr.appendChild(simpleRowHtml(formatRetransmit(row.rxmit_down))); + tr.appendChild(simpleRowTrustedHtml(formatRetransmit(row.rxmit_down))); } if (row.rxmit_up === null) { - tr.appendChild(simpleRowHtml("-")); + tr.appendChild(simpleRowTrustedHtml("-")); } else { - tr.appendChild(simpleRowHtml(formatRetransmit(row.rxmit_up))); + tr.appendChild(simpleRowTrustedHtml(formatRetransmit(row.rxmit_up))); } const asnCell = document.createElement("td"); asnCell.classList.add("lqos-asn-cell"); diff --git a/src/rust/lqosd/src/node_manager/js_build/src/dashlets/top_tree_summary.js b/src/rust/lqosd/src/node_manager/js_build/src/dashlets/top_tree_summary.js index 7eb3e231..cfc47aea 100644 --- a/src/rust/lqosd/src/node_manager/js_build/src/dashlets/top_tree_summary.js +++ b/src/rust/lqosd/src/node_manager/js_build/src/dashlets/top_tree_summary.js @@ -1,4 +1,4 @@ -import {clearDiv, simpleRowHtml, theading} from "../helpers/builders"; +import {clearDiv, simpleRowTrustedHtml, theading} from "../helpers/builders"; import {formatThroughput, formatRetransmit, formatCakeStat} from "../helpers/scaling"; import {toNumber} from "../lq_js_common/helpers/scaling"; import {DashletBaseInsight} from "./insight_dashlet_base"; @@ -89,8 +89,8 @@ export class TopTreeSummary extends DashletBaseInsight { const tpDown = toNumber(r[1].current_throughput[0], 0) * 8; const tpUp = toNumber(r[1].current_throughput[1], 0) * 8; const maxThroughput = effectiveMax(r[1]); - row.appendChild(simpleRowHtml(formatThroughput(tpDown, maxThroughput[0]))); - row.appendChild(simpleRowHtml(formatThroughput(tpUp, maxThroughput[1]))); + row.appendChild(simpleRowTrustedHtml(formatThroughput(tpDown, maxThroughput[0]))); + row.appendChild(simpleRowTrustedHtml(formatThroughput(tpUp, maxThroughput[1]))); const tcpPacketsDown = retransmitPacketsForNode(r[1], 0); const tcpPacketsUp = retransmitPacketsForNode(r[1], 1); @@ -98,19 +98,19 @@ export class TopTreeSummary extends DashletBaseInsight { const retransmitsUp = toNumber(r[1].current_retransmits[1], 0); if (tcpPacketsDown > 0) { - row.appendChild(simpleRowHtml(formatRetransmit(retransmitsDown / tcpPacketsDown))) + row.appendChild(simpleRowTrustedHtml(formatRetransmit(retransmitsDown / tcpPacketsDown))) } else { - row.appendChild(simpleRowHtml("")); + row.appendChild(simpleRowTrustedHtml("")); } if (tcpPacketsUp > 0) { - row.appendChild(simpleRowHtml(formatRetransmit(retransmitsUp / tcpPacketsUp))) + row.appendChild(simpleRowTrustedHtml(formatRetransmit(retransmitsUp / tcpPacketsUp))) } else { - row.appendChild(simpleRowHtml("")); + row.appendChild(simpleRowTrustedHtml("")); } - row.appendChild(simpleRowHtml(formatCakeStat(r[1].current_marks[0]))) - row.appendChild(simpleRowHtml(formatCakeStat(r[1].current_marks[1]))) - row.appendChild(simpleRowHtml(formatCakeStat(r[1].current_drops[0]))) - row.appendChild(simpleRowHtml(formatCakeStat(r[1].current_drops[1]))) + row.appendChild(simpleRowTrustedHtml(formatCakeStat(r[1].current_marks[0]))) + row.appendChild(simpleRowTrustedHtml(formatCakeStat(r[1].current_marks[1]))) + row.appendChild(simpleRowTrustedHtml(formatCakeStat(r[1].current_drops[0]))) + row.appendChild(simpleRowTrustedHtml(formatCakeStat(r[1].current_drops[1]))) target.appendChild(row); }); } diff --git a/src/rust/lqosd/src/node_manager/js_build/src/dashlets/tree_capacity_dash.js b/src/rust/lqosd/src/node_manager/js_build/src/dashlets/tree_capacity_dash.js index 308d379f..e70567b4 100644 --- a/src/rust/lqosd/src/node_manager/js_build/src/dashlets/tree_capacity_dash.js +++ b/src/rust/lqosd/src/node_manager/js_build/src/dashlets/tree_capacity_dash.js @@ -1,4 +1,4 @@ -import {clearDashDiv, simpleRowHtml, theading} from "../helpers/builders"; +import {clearDashDiv, simpleRowTrustedHtml, theading} from "../helpers/builders"; import {formatRtt, formatPercent} from "../helpers/scaling"; import {DashletBaseInsight} from "./insight_dashlet_base"; @@ -74,9 +74,9 @@ export class TreeCapacityDash extends DashletBaseInsight { linkCol.appendChild(link); row.appendChild(linkCol); - row.appendChild(simpleRowHtml(formatPercent(down*100))); - row.appendChild(simpleRowHtml(formatPercent(up*100))); - row.appendChild(simpleRowHtml(formatRtt(node.rtt))); + row.appendChild(simpleRowTrustedHtml(formatPercent(down*100))); + row.appendChild(simpleRowTrustedHtml(formatPercent(up*100))); + row.appendChild(simpleRowTrustedHtml(formatRtt(node.rtt))); tbody.appendChild(row); }); diff --git a/src/rust/lqosd/src/node_manager/js_build/src/dashlets/worst10_downloaders.js b/src/rust/lqosd/src/node_manager/js_build/src/dashlets/worst10_downloaders.js index 85c46f07..aebf4643 100644 --- a/src/rust/lqosd/src/node_manager/js_build/src/dashlets/worst10_downloaders.js +++ b/src/rust/lqosd/src/node_manager/js_build/src/dashlets/worst10_downloaders.js @@ -1,4 +1,11 @@ -import {clearDashDiv, simpleRow, simpleRowHtml, theading, TopNTableFromMsgData} from "../helpers/builders"; +import { + clearDashDiv, + simpleLinkRow, + simpleRow, + simpleRowTrustedHtml, + theading, + TopNTableFromMsgData, +} from "../helpers/builders"; import {TimedCache} from "../lq_js_common/helpers/timed_cache"; import {periodNameToSeconds} from "../helpers/time_periods"; import {formatRetransmit, formatRtt} from "../helpers/scaling"; @@ -94,15 +101,19 @@ export class Worst10Downloaders extends DashletBaseInsight { //console.log(row); let tr = document.createElement("tr"); tr.classList.add("small"); - tr.appendChild(simpleRowHtml("" + row.circuit_name + "")); + tr.appendChild(simpleLinkRow( + `circuit.html?circuit=${encodeURIComponent(row.circuit_hash ?? "")}`, + row.circuit_name, + true, + )); tr.appendChild(simpleRow(scaleNumber(toNumber(row.bytes_down, 0) * 1000000, 0))); if (row.rtt !== null) { - tr.appendChild(simpleRowHtml(formatRtt(row.rtt))); + tr.appendChild(simpleRowTrustedHtml(formatRtt(row.rtt))); } else { tr.appendChild(simpleRow("-")); } if (row.rxmit !== null) { - tr.appendChild(simpleRowHtml(formatRetransmit(row.rxmit))); + tr.appendChild(simpleRowTrustedHtml(formatRetransmit(row.rxmit))); } else { tr.appendChild(simpleRow("-")); } diff --git a/src/rust/lqosd/src/node_manager/js_build/src/dashlets/worst10_retransmits.js b/src/rust/lqosd/src/node_manager/js_build/src/dashlets/worst10_retransmits.js index 7d9d8730..84211db2 100644 --- a/src/rust/lqosd/src/node_manager/js_build/src/dashlets/worst10_retransmits.js +++ b/src/rust/lqosd/src/node_manager/js_build/src/dashlets/worst10_retransmits.js @@ -1,4 +1,11 @@ -import {clearDashDiv, simpleRow, simpleRowHtml, theading, TopNTableFromMsgData} from "../helpers/builders"; +import { + clearDashDiv, + simpleLinkRow, + simpleRow, + simpleRowTrustedHtml, + theading, + TopNTableFromMsgData, +} from "../helpers/builders"; import {TimedCache} from "../lq_js_common/helpers/timed_cache"; import {periodNameToSeconds} from "../helpers/time_periods"; import {formatRetransmit, formatRtt, retransmitFractionFromSample} from "../helpers/scaling"; @@ -98,15 +105,19 @@ export class Worst10Retransmits extends DashletBaseInsight { //console.log(row); let tr = document.createElement("tr"); tr.classList.add("small"); - tr.appendChild(simpleRowHtml("" + row.circuit_name + "")); + tr.appendChild(simpleLinkRow( + `circuit.html?circuit=${encodeURIComponent(row.circuit_hash ?? "")}`, + row.circuit_name, + true, + )); tr.appendChild(simpleRow(scaleNumber(toNumber(row.bytes_down, 0) * 1000000, 0))); if (row.rtt !== null) { - tr.appendChild(simpleRowHtml(formatRtt(row.rtt))); + tr.appendChild(simpleRowTrustedHtml(formatRtt(row.rtt))); } else { tr.appendChild(simpleRow("-")); } if (row.rxmit !== null) { - tr.appendChild(simpleRowHtml(formatRetransmit(row.rxmit))); + tr.appendChild(simpleRowTrustedHtml(formatRetransmit(row.rxmit))); } else { tr.appendChild(simpleRow("-")); } diff --git a/src/rust/lqosd/src/node_manager/js_build/src/first-run.js b/src/rust/lqosd/src/node_manager/js_build/src/first-run.js index a605adb1..d435fe6b 100644 --- a/src/rust/lqosd/src/node_manager/js_build/src/first-run.js +++ b/src/rust/lqosd/src/node_manager/js_build/src/first-run.js @@ -1,7 +1,6 @@ $("#btnCreateUser").on('click', () => { let username = ($("#username").val() || "").trim(); let password = $("#password").val(); - let anon = document.getElementById("allowAnonymous").checked; if (username === "") { alert("You must enter a username"); return; @@ -12,7 +11,6 @@ $("#btnCreateUser").on('click', () => { } let login = { - allow_anonymous: anon, username: username, password: password } diff --git a/src/rust/lqosd/src/node_manager/js_build/src/helpers/builders.js b/src/rust/lqosd/src/node_manager/js_build/src/helpers/builders.js index b73bb3cb..320b3791 100644 --- a/src/rust/lqosd/src/node_manager/js_build/src/helpers/builders.js +++ b/src/rust/lqosd/src/node_manager/js_build/src/helpers/builders.js @@ -7,6 +7,7 @@ import { import {scaleNanos} from "../lq_js_common/helpers/scaling"; import {redactCell} from "./redact"; import {disposeTooltipsWithin, enableTooltipsWithin} from "../lq_js_common/helpers/tooltips"; +export {safeRelativeHref, simpleLinkRow} from "./safe_dom.mjs"; export function heading5Icon(icon, text) { let h5 = document.createElement("h5"); @@ -42,7 +43,7 @@ export function simpleRow(text, redact=false) { return td; } -export function simpleRowHtml(text) { +export function simpleRowTrustedHtml(text) { let td = document.createElement("td"); td.innerHTML = text; return td; diff --git a/src/rust/lqosd/src/node_manager/js_build/src/helpers/builders.test.mjs b/src/rust/lqosd/src/node_manager/js_build/src/helpers/builders.test.mjs new file mode 100644 index 00000000..123017fa --- /dev/null +++ b/src/rust/lqosd/src/node_manager/js_build/src/helpers/builders.test.mjs @@ -0,0 +1,103 @@ +import assert from "node:assert/strict"; +import test from "node:test"; + +import {appendIconText, safeRelativeHref, simpleLinkRow} from "./safe_dom.mjs"; + +class FakeClassList { + constructor() { + this.values = []; + } + + add(...classes) { + this.values.push(...classes); + } + + contains(className) { + return this.values.includes(className); + } +} + +class FakeElement { + constructor(tagName) { + this.tagName = tagName.toUpperCase(); + this.children = []; + this.classList = new FakeClassList(); + this._textContent = ""; + this._href = ""; + this.attributes = {}; + } + + appendChild(child) { + this.children.push(child); + } + + setAttribute(name, value) { + this.attributes[name] = String(value); + } + + set href(value) { + this._href = String(value); + } + + get href() { + return this._href; + } + + set textContent(value) { + this._textContent = String(value); + } + + get textContent() { + return this._textContent; + } +} + +class FakeTextNode { + constructor(text) { + this.textContent = String(text); + } +} + +test("safeRelativeHref rejects scriptable URL protocols", () => { + assert.equal(safeRelativeHref("javascript:alert(1)"), "#"); + assert.equal(safeRelativeHref(" data:text/html,boom"), "#"); + assert.equal(safeRelativeHref("https://example.invalid/circuit"), "#"); + assert.equal(safeRelativeHref("//example.invalid/circuit"), "#"); + assert.equal(safeRelativeHref("circuit.html?circuit=123"), "circuit.html?circuit=123"); +}); + +test("simpleLinkRow renders operator text as text content", () => { + globalThis.document = { + createElement: (tagName) => new FakeElement(tagName), + }; + + const row = simpleLinkRow( + "javascript:alert(1)", + "", + true, + ); + const link = row.children[0]; + + assert.equal(row.tagName, "TD"); + assert.equal(link.tagName, "A"); + assert.equal(link.href, "#"); + assert.equal(link.textContent, ""); + assert.equal(link.classList.contains("redactable"), true); +}); + +test("appendIconText appends operator text without innerHTML", () => { + globalThis.document = { + createElement: (tagName) => new FakeElement(tagName), + createTextNode: (textValue) => new FakeTextNode(textValue), + }; + + const link = new FakeElement("a"); + appendIconText(link, ["fa", "fa-save"], ""); + + assert.equal(link.children.length, 3); + assert.equal(link.children[0].tagName, "I"); + assert.equal(link.children[0].attributes["aria-hidden"], "true"); + assert.deepEqual(link.children[0].classList.values, ["fa", "fa-save"]); + assert.equal(link.children[1].textContent, " "); + assert.equal(link.children[2].textContent, ""); +}); diff --git a/src/rust/lqosd/src/node_manager/js_build/src/helpers/safe_dom.mjs b/src/rust/lqosd/src/node_manager/js_build/src/helpers/safe_dom.mjs new file mode 100644 index 00000000..065145e2 --- /dev/null +++ b/src/rust/lqosd/src/node_manager/js_build/src/helpers/safe_dom.mjs @@ -0,0 +1,29 @@ +export function safeRelativeHref(href) { + const value = String(href ?? ""); + const trimmed = value.trimStart(); + if (/^[a-z][a-z0-9+.-]*:/i.test(trimmed) || trimmed.startsWith("//")) { + return "#"; + } + return value; +} + +export function simpleLinkRow(href, text, redact=false) { + let td = document.createElement("td"); + let link = document.createElement("a"); + link.href = safeRelativeHref(href); + link.textContent = text ?? ""; + if (redact) { + link.classList.add("redactable"); + } + td.appendChild(link); + return td; +} + +export function appendIconText(target, iconClasses, text) { + const icon = document.createElement("i"); + icon.classList.add(...iconClasses); + icon.setAttribute("aria-hidden", "true"); + target.appendChild(icon); + target.appendChild(document.createTextNode(" ")); + target.appendChild(document.createTextNode(text ?? "")); +} diff --git a/src/rust/lqosd/src/node_manager/js_build/src/lq_js_common/dashboard/dashboard.js b/src/rust/lqosd/src/node_manager/js_build/src/lq_js_common/dashboard/dashboard.js index ba650d04..f0674017 100644 --- a/src/rust/lqosd/src/node_manager/js_build/src/lq_js_common/dashboard/dashboard.js +++ b/src/rust/lqosd/src/node_manager/js_build/src/lq_js_common/dashboard/dashboard.js @@ -3,6 +3,7 @@ import {DashboardLayout} from "./layout"; import {get_ws_client} from "../../pubsub/ws"; import {heading5Icon} from "../helpers/content_builders"; import {openDashboardEditor} from "./dashboard_editor"; +import {appendIconText} from "../../helpers/safe_dom.mjs"; const DIAGNOSTIC_CHANNELS = new Set(["Cpu", "Ram", "RttHistogram"]); const DASHBOARD_DATA_STALE_MS = 15000; @@ -1021,7 +1022,7 @@ export class Dashboard { i.classList.add("list-group-item","list-group-item-action"); let ln = document.createElement("a"); ln.href = "#"; - ln.innerHTML = " " + d.name; + appendIconText(ln, ["fa", "fa-save"], d.name); ln.onclick = () => { let resp = confirm("Load [" + d.name + "] from [" + d.path + "]?"); if (resp) { diff --git a/src/rust/lqosd/src/node_manager/js_build/src/pubsub/ws.js b/src/rust/lqosd/src/node_manager/js_build/src/pubsub/ws.js index 59ed4248..a8667aaa 100644 --- a/src/rust/lqosd/src/node_manager/js_build/src/pubsub/ws.js +++ b/src/rust/lqosd/src/node_manager/js_build/src/pubsub/ws.js @@ -1,8 +1,8 @@ import { Encoder, decode } from "../lq_js_common/helpers/cbor-x"; +import { websocketHelloReply } from "./ws_auth.mjs"; const ACK_TEXT = "I accept that this is an unstable, internal API and is unsupported"; const EXPECTED_UI_VERSION = (window.LQOS_UI_VERSION || "").trim() || null; -const USER_TOKEN_COOKIE = "User-Token"; const VERSION_RELOAD_KEY = "lqosWsVersionReload"; const encoder = new Encoder({ useRecords: false, variableMapSize: true }); const DIAGNOSTIC_CHANNELS = new Set(["Cpu", "Ram", "RttHistogram"]); @@ -63,22 +63,6 @@ function dashboardWsInteresting(event, channels = [], details = {}) { }); } -function get_cookie_value(name) { - const cookies = document.cookie ? document.cookie.split(";") : []; - const prefix = `${name}=`; - for (let i = 0; i < cookies.length; i++) { - const entry = cookies[i].trim(); - if (entry.startsWith(prefix)) { - return decodeURIComponent(entry.slice(prefix.length)); - } - } - return ""; -} - -function get_user_token() { - return get_cookie_value(USER_TOKEN_COOKIE); -} - function versionReloadKey(serverVersion) { const clientVersion = EXPECTED_UI_VERSION || "missing"; const wsVersion = (serverVersion || "missing").toString().trim() || "missing"; @@ -419,12 +403,7 @@ export class WsClient { dashboardWsDebug("handshake-complete", { desiredChannels: Array.from(this.desiredChannels.keys()), }); - if (!this._sendControl({ - HelloReply: { - ack: ACK_TEXT, - token: get_user_token(), - }, - })) { + if (!this._sendControl(websocketHelloReply(ACK_TEXT))) { return; } const pending = this.pending; diff --git a/src/rust/lqosd/src/node_manager/js_build/src/pubsub/ws.test.mjs b/src/rust/lqosd/src/node_manager/js_build/src/pubsub/ws.test.mjs index 46622629..eb76cb7d 100644 --- a/src/rust/lqosd/src/node_manager/js_build/src/pubsub/ws.test.mjs +++ b/src/rust/lqosd/src/node_manager/js_build/src/pubsub/ws.test.mjs @@ -101,3 +101,20 @@ test("timeout listener fires timeout and removes pending handler", async () => { assert.deepEqual(seen, ["timeout"]); assert.equal(wsClient.handlerCount("UrgentList"), 0); }); + +test("websocket handshake does not copy the session cookie into auth payload", async () => { + let cookieRead = false; + Object.defineProperty(globalThis.document, "cookie", { + configurable: true, + get() { + cookieRead = true; + return "User-Token=v1.secret.signature"; + }, + }); + + const { websocketHelloReply } = await import("./ws_auth.mjs"); + const reply = websocketHelloReply("ack"); + + assert.equal(cookieRead, false); + assert.equal(reply.HelloReply.token, ""); +}); diff --git a/src/rust/lqosd/src/node_manager/js_build/src/pubsub/ws_auth.mjs b/src/rust/lqosd/src/node_manager/js_build/src/pubsub/ws_auth.mjs new file mode 100644 index 00000000..8c76abbf --- /dev/null +++ b/src/rust/lqosd/src/node_manager/js_build/src/pubsub/ws_auth.mjs @@ -0,0 +1,8 @@ +export function websocketHelloReply(ackText) { + return { + HelloReply: { + ack: ackText, + token: "", + }, + }; +} diff --git a/src/rust/lqosd/src/node_manager/js_build/src/template.js b/src/rust/lqosd/src/node_manager/js_build/src/template.js index 62bdb72c..1ba8df84 100644 --- a/src/rust/lqosd/src/node_manager/js_build/src/template.js +++ b/src/rust/lqosd/src/node_manager/js_build/src/template.js @@ -1,9 +1,10 @@ -import {clearDiv} from "./helpers/builders"; +import {clearDiv, safeRelativeHref} from "./helpers/builders"; import {initRedact} from "./helpers/redact"; import {initDayNightMode} from "./helpers/dark_mode"; import {initColorBlind} from "./helpers/colorblind"; import {listenOnceMatchingWithTimeout as listenWsOnceMatchingWithTimeout} from "./pubsub/listeners"; import {get_ws_client} from "./pubsub/ws"; +import {clearOperationalNetworkModeStorage} from "./config/network_mode_storage.mjs"; const wsClient = get_ws_client(); const listenOnce = (eventName, handler) => { @@ -752,6 +753,7 @@ function getDeviceCounts() { function initLogout() { $("#btnLogout").on('click', () => { //console.log("Logout"); + clearOperationalNetworkModeStorage(); const cookies = document.cookie.split(";"); for (let i = 0; i < cookies.length; i++) { @@ -794,11 +796,23 @@ function renderSearchResults(data) { let c = document.createElement("td"); if (item.Circuit !== undefined) { - c.innerHTML = " " + item.Circuit.name + ""; + c.appendChild(searchResultLink( + `/circuit.html?id=${encodeURIComponent(item.Circuit.id ?? "")}`, + ["fa", "fa-user"], + item.Circuit.name, + )); } else if (item.Device !== undefined) { - c.innerHTML = " " + item.Device.name + ""; + c.appendChild(searchResultLink( + `/circuit.html?id=${encodeURIComponent(item.Device.circuit_id ?? "")}`, + ["fa", "fa-computer"], + item.Device.name, + )); } else if (item.Site !== undefined) { - c.innerHTML = " " + item.Site.name + ""; + c.appendChild(searchResultLink( + `/tree.html?parent=${encodeURIComponent(item.Site.idx ?? "")}`, + ["fa", "fa-building"], + item.Site.name, + )); } else { console.log(item); c.innerText = item; @@ -814,6 +828,18 @@ function renderSearchResults(data) { searchResults.appendChild(wrap); } +function searchResultLink(href, iconClasses, label) { + const link = document.createElement("a"); + link.classList.add("nav-link", "redactable"); + link.href = safeRelativeHref(href); + + const icon = document.createElement("i"); + icon.classList.add(...iconClasses); + link.appendChild(icon); + link.appendChild(document.createTextNode(` ${label ?? ""}`)); + return link; +} + function doSearch(search) { if (search.length > 2) { lastSearchTerm = search; diff --git a/src/rust/lqosd/src/node_manager/js_build/src/tree.js b/src/rust/lqosd/src/node_manager/js_build/src/tree.js index e3f4827c..e5e545f8 100644 --- a/src/rust/lqosd/src/node_manager/js_build/src/tree.js +++ b/src/rust/lqosd/src/node_manager/js_build/src/tree.js @@ -1,4 +1,4 @@ -import {clearDiv, clientTableHeader, formatLastSeen, simpleRow, simpleRowHtml, theading} from "./helpers/builders"; +import {clearDiv, clientTableHeader, formatLastSeen, simpleRow, simpleRowTrustedHtml, theading} from "./helpers/builders"; import { formatCakeStat, formatCakeStatPercent, formatRetransmit, formatRetransmitFraction, retransmitFractionFromSample, @@ -1870,22 +1870,22 @@ function renderAttachedCircuitsRows(rows) { tr.appendChild(ipCell); tr.appendChild(simpleRow(formatLastSeen(toNumber(circuit.last_seen_nanos, 0)))); - tr.appendChild(simpleRowHtml(formatThroughput(toNumber(circuit.bytes_per_second?.down, 0) * 8, toNumber(circuit.plan_mbps?.down, 0)))); - tr.appendChild(simpleRowHtml(formatThroughput(toNumber(circuit.bytes_per_second?.up, 0) * 8, toNumber(circuit.plan_mbps?.up, 0)))); + tr.appendChild(simpleRowTrustedHtml(formatThroughput(toNumber(circuit.bytes_per_second?.down, 0) * 8, toNumber(circuit.plan_mbps?.down, 0)))); + tr.appendChild(simpleRowTrustedHtml(formatThroughput(toNumber(circuit.bytes_per_second?.up, 0) * 8, toNumber(circuit.plan_mbps?.up, 0)))); if (toNumber(circuit.rtt_current_p50_nanos?.down, 0) > 0) { - tr.appendChild(simpleRowHtml(formatRtt(toNumber(circuit.rtt_current_p50_nanos?.down, 0) / 1_000_000))); + tr.appendChild(simpleRowTrustedHtml(formatRtt(toNumber(circuit.rtt_current_p50_nanos?.down, 0) / 1_000_000))); } else { tr.appendChild(simpleRow("-")); } if (toNumber(circuit.rtt_current_p50_nanos?.up, 0) > 0) { - tr.appendChild(simpleRowHtml(formatRtt(toNumber(circuit.rtt_current_p50_nanos?.up, 0) / 1_000_000))); + tr.appendChild(simpleRowTrustedHtml(formatRtt(toNumber(circuit.rtt_current_p50_nanos?.up, 0) / 1_000_000))); } else { tr.appendChild(simpleRow("-")); } - tr.appendChild(simpleRowHtml(formatRetransmit(retransmitFractionFromSample(circuit.tcp_retransmit_sample?.down)))); - tr.appendChild(simpleRowHtml(formatRetransmit(retransmitFractionFromSample(circuit.tcp_retransmit_sample?.up)))); + tr.appendChild(simpleRowTrustedHtml(formatRetransmit(retransmitFractionFromSample(circuit.tcp_retransmit_sample?.down)))); + tr.appendChild(simpleRowTrustedHtml(formatRetransmit(retransmitFractionFromSample(circuit.tcp_retransmit_sample?.up)))); tbody.appendChild(tr); }); diff --git a/src/rust/lqosd/src/node_manager/js_build/test-build-contract.sh b/src/rust/lqosd/src/node_manager/js_build/test-build-contract.sh index 0740be56..0e80bd29 100755 --- a/src/rust/lqosd/src/node_manager/js_build/test-build-contract.sh +++ b/src/rust/lqosd/src/node_manager/js_build/test-build-contract.sh @@ -51,6 +51,21 @@ for page in "${served_pages[@]}"; do [[ -f "${STATIC_DIR}/${page}" ]] || fail "Page is served by static_pages.rs but missing from static2: ${page}" done +declare -A directly_handled_html=( + ["first-run.html"]=1 + ["login.html"]=1 + ["template.html"]=1 +) + +for html_path in "${STATIC_DIR}"/*.html; do + [[ -e "${html_path}" ]] || continue + page="$(basename "${html_path}")" + [[ -n "${directly_handled_html[${page}]:-}" ]] && continue + if ! printf '%s\n' "${served_pages[@]}" | grep -qx -- "${page}"; then + fail "HTML page is present in static2 but not in the authenticated static_pages.rs route list: ${page}" + fi +done + required_template_assets=( "node_manager.css" "vendor/bootstrap.min.css" diff --git a/src/rust/lqosd/src/node_manager/js_build/test-node-manager.sh b/src/rust/lqosd/src/node_manager/js_build/test-node-manager.sh index 85f5d6de..2a24d2f5 100755 --- a/src/rust/lqosd/src/node_manager/js_build/test-node-manager.sh +++ b/src/rust/lqosd/src/node_manager/js_build/test-node-manager.sh @@ -10,8 +10,11 @@ fi echo "[test-node-manager] Running frontend contract tests..." node --test \ + "${SCRIPT_DIR}/src/helpers/builders.test.mjs" \ + "${SCRIPT_DIR}/src/circuit_packet_capture_dom.test.mjs" \ "${SCRIPT_DIR}/src/config/shaped_device_wire.test.mjs" \ "${SCRIPT_DIR}/src/config/config_interface_mtu.test.mjs" \ + "${SCRIPT_DIR}/src/config/network_mode_storage.test.mjs" \ "${SCRIPT_DIR}/src/config/ssl_redirect.test.mjs" \ "${SCRIPT_DIR}/src/pubsub/ws.test.mjs" diff --git a/src/rust/lqosd/src/node_manager/local_api.rs b/src/rust/lqosd/src/node_manager/local_api.rs index af0d02ad..e9d4d519 100644 --- a/src/rust/lqosd/src/node_manager/local_api.rs +++ b/src/rust/lqosd/src/node_manager/local_api.rs @@ -37,7 +37,6 @@ use crate::node_manager::auth::auth_layer; use crate::node_manager::shaper_queries_actor::ShaperQueryCommand; use axum::routing::{get, post}; use axum::{Extension, Router}; -use tower_http::cors::CorsLayer; pub fn local_api(shaper_query: tokio::sync::mpsc::Sender) -> Router { Router::new() @@ -62,6 +61,5 @@ pub fn local_api(shaper_query: tokio::sync::mpsc::Sender) -> .route("/ssl/disable", post(ssl::disable)) .with_state(network_mode::NetworkModeApiState::default()) .layer(Extension(shaper_query)) - .layer(CorsLayer::very_permissive()) .route_layer(axum::middleware::from_fn(auth_layer)) } diff --git a/src/rust/lqosd/src/node_manager/local_api/dashboard_themes.rs b/src/rust/lqosd/src/node_manager/local_api/dashboard_themes.rs index 09754cfd..19a0f162 100644 --- a/src/rust/lqosd/src/node_manager/local_api/dashboard_themes.rs +++ b/src/rust/lqosd/src/node_manager/local_api/dashboard_themes.rs @@ -2,6 +2,8 @@ use serde::{Deserialize, Serialize}; use lqos_config::load_config; +const DASHBOARD_THEME_NAME_MAX_CHARS: usize = 64; + #[derive(Serialize, Deserialize, Debug, Clone)] pub struct DashletSave { pub name: String, @@ -32,13 +34,37 @@ fn dashboards_dir() -> Option { Some(base_path) } -fn normalize_theme_filename(name: &str) -> String { - let safe = name.replace(['/', '\\'], "_"); - if safe.ends_with(".json") { - safe - } else { - format!("{}.json", safe) +fn theme_name_stem(name: &str) -> &str { + let trimmed = name.trim(); + trimmed.strip_suffix(".json").unwrap_or(trimmed) +} + +fn is_allowed_theme_name_char(ch: char) -> bool { + ch.is_ascii_alphanumeric() || matches!(ch, ' ' | '.' | '_' | '-' | '(' | ')' | '[' | ']') +} + +fn validate_theme_name(name: &str) -> Result { + let stem = theme_name_stem(name); + if stem.is_empty() { + return Err("Dashboard layout name is required.".to_string()); } + if stem.chars().count() > DASHBOARD_THEME_NAME_MAX_CHARS { + return Err(format!( + "Dashboard layout names must be {DASHBOARD_THEME_NAME_MAX_CHARS} characters or fewer." + )); + } + if !stem.chars().all(is_allowed_theme_name_char) { + return Err( + "Dashboard layout names may only use letters, numbers, spaces, dots, underscores, hyphens, parentheses, and brackets." + .to_string(), + ); + } + Ok(stem.to_string()) +} + +fn theme_filename(name: &str) -> Result { + let safe_name = validate_theme_name(name)?; + Ok(format!("{safe_name}.json")) } pub(crate) fn list_theme_entries() -> Vec { @@ -53,16 +79,21 @@ pub(crate) fn list_theme_entries() -> Vec { }; for f in entries.flatten() { - let fs = f.file_name().to_string_lossy().to_string(); - if !fs.ends_with(".json") { + let Some(fs) = f.file_name().to_str().map(str::to_string) else { continue; - } - let mut display_name = fs.trim_end_matches(".json").to_string(); + }; + let Some(stem) = fs.strip_suffix(".json") else { + continue; + }; + let Ok(mut display_name) = validate_theme_name(stem) else { + continue; + }; if let Ok(raw) = std::fs::read_to_string(f.path()) && let Ok(parsed) = serde_json::from_str::(&raw) && !parsed.name.is_empty() + && let Ok(valid_name) = validate_theme_name(&parsed.name) { - display_name = parsed.name; + display_name = valid_name; } result.push(ThemeEntry { name: display_name, @@ -77,7 +108,9 @@ pub(crate) fn load_theme_entries(name: &str) -> Vec { return Vec::new(); }; - let filename = normalize_theme_filename(name); + let Ok(filename) = theme_filename(name) else { + return Vec::new(); + }; let file_path = base_path.join(filename); if !file_path.exists() { return Vec::new(); @@ -96,19 +129,58 @@ pub(crate) fn load_theme_entries(name: &str) -> Vec { pub(crate) fn save_theme_data(data: &DashletSave) -> Result<(), String> { let base_path = dashboards_dir().ok_or_else(|| "Unable to load configuration".to_string())?; - let filename = normalize_theme_filename(&data.name); + let valid_name = validate_theme_name(&data.name)?; + let filename = theme_filename(&valid_name)?; let file_path = base_path.join(filename); - let serialized = serde_json::to_string(data).map_err(|e| format!("Serialize error: {e}"))?; + let sanitized = DashletSave { + name: valid_name, + entries: data.entries.clone(), + }; + let serialized = + serde_json::to_string(&sanitized).map_err(|e| format!("Serialize error: {e}"))?; std::fs::write(&file_path, serialized.as_bytes()).map_err(|e| format!("Write error: {e}"))?; Ok(()) } pub(crate) fn delete_theme_file(name: &str) -> Result<(), String> { let base_path = dashboards_dir().ok_or_else(|| "Unable to load configuration".to_string())?; - let filename = normalize_theme_filename(name); + let filename = theme_filename(name)?; let file_path = base_path.join(filename); if file_path.exists() { std::fs::remove_file(file_path).map_err(|e| format!("Delete error: {e}"))?; } Ok(()) } + +#[cfg(test)] +mod tests { + use super::{theme_filename, validate_theme_name}; + + #[test] + fn dashboard_theme_names_allow_operator_friendly_labels() { + assert_eq!( + validate_theme_name(" Evening Layout 1 [NOC] ").expect("valid layout name"), + "Evening Layout 1 [NOC]" + ); + assert_eq!( + theme_filename("Primary-Dashboard.json").expect("valid filename"), + "Primary-Dashboard.json" + ); + } + + #[test] + fn dashboard_theme_names_reject_paths_and_markup() { + assert!(validate_theme_name("").is_err()); + assert!(validate_theme_name(" ").is_err()); + assert!(validate_theme_name("../operator").is_err()); + assert!(validate_theme_name("operator/layout").is_err()); + assert!(validate_theme_name("").is_err()); + assert!(validate_theme_name("layout & alert").is_err()); + } + + #[test] + fn dashboard_theme_names_enforce_length_limit() { + assert!(validate_theme_name(&"a".repeat(64)).is_ok()); + assert!(validate_theme_name(&"a".repeat(65)).is_err()); + } +} diff --git a/src/rust/lqosd/src/node_manager/local_api/flow_explorer.rs b/src/rust/lqosd/src/node_manager/local_api/flow_explorer.rs index 87444806..465dc98b 100644 --- a/src/rust/lqosd/src/node_manager/local_api/flow_explorer.rs +++ b/src/rust/lqosd/src/node_manager/local_api/flow_explorer.rs @@ -5,7 +5,7 @@ use crate::throughput_tracker::flow_data::{ use crate::throughput_tracker::resolve_circuit_metadata_for_ip; use lqos_sys::flowbee_data::FlowbeeKey; use lqos_utils::units::DownUpOrder; -use lqos_utils::unix_time::{time_since_boot, unix_now}; +use lqos_utils::unix_time::{TimeError, time_since_boot, unix_now}; use serde::Serialize; use std::time::Duration; @@ -38,16 +38,24 @@ pub struct FlowTimeline { pub remote_ip: String, } -pub fn flow_timeline_data(asn_id: u32) -> Vec { - let time_since_boot = time_since_boot().expect("failed to retrieve time since boot"); - let since_boot = Duration::from(time_since_boot); - let boot_time = unix_now() - .expect("failed to retrieve current unix time") - .saturating_sub(since_boot.as_secs()); +pub fn flow_timeline_data(asn_id: u32) -> Result, TimeError> { + let boot_time = flow_timeline_boot_time()?; let all_flows_for_asn = RECENT_FLOWS.all_flows_for_asn(asn_id); - all_flows_to_transport(boot_time, all_flows_for_asn) + Ok(all_flows_to_transport(boot_time, all_flows_for_asn)) +} + +fn flow_timeline_boot_time() -> Result { + flow_timeline_boot_time_from(|| time_since_boot().map(Duration::from), unix_now) +} + +fn flow_timeline_boot_time_from( + since_boot: impl FnOnce() -> Result, + now: impl FnOnce() -> Result, +) -> Result { + let since_boot = since_boot()?; + Ok(now()?.saturating_sub(since_boot.as_secs())) } fn all_flows_to_transport( @@ -100,27 +108,39 @@ fn all_flows_to_transport( .collect::>() } -pub fn country_timeline_data(iso_code: &str) -> Vec { - let time_since_boot = time_since_boot().expect("failed to retrieve time since boot"); - let since_boot = Duration::from(time_since_boot); - let boot_time = unix_now() - .expect("failed to retrieve current unix time") - .saturating_sub(since_boot.as_secs()); +pub fn country_timeline_data(iso_code: &str) -> Result, TimeError> { + let boot_time = flow_timeline_boot_time()?; let all_flows_for_asn = RECENT_FLOWS.all_flows_for_country(iso_code); - all_flows_to_transport(boot_time, all_flows_for_asn) + Ok(all_flows_to_transport(boot_time, all_flows_for_asn)) } -pub fn protocol_timeline_data(protocol_name: &str) -> Vec { +pub fn protocol_timeline_data(protocol_name: &str) -> Result, TimeError> { let protocol_name = protocol_name.replace("_", "/"); - let time_since_boot = time_since_boot().expect("failed to retrieve time since boot"); - let since_boot = Duration::from(time_since_boot); - let boot_time = unix_now() - .expect("failed to retrieve current unix time") - .saturating_sub(since_boot.as_secs()); + let boot_time = flow_timeline_boot_time()?; let all_flows_for_asn = RECENT_FLOWS.all_flows_for_protocol(&protocol_name); - all_flows_to_transport(boot_time, all_flows_for_asn) + Ok(all_flows_to_transport(boot_time, all_flows_for_asn)) +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn boot_time_helper_propagates_clock_errors() { + let result = flow_timeline_boot_time_from(|| Err(TimeError::ClockNotReady), || Ok(1_000)); + + assert!(matches!(result, Err(TimeError::ClockNotReady))); + } + + #[test] + fn boot_time_helper_saturates_underflow() { + let boot_time = flow_timeline_boot_time_from(|| Ok(Duration::from_secs(20)), || Ok(10)) + .expect("test clock values should build a boot time"); + + assert_eq!(boot_time, 0); + } } diff --git a/src/rust/lqosd/src/node_manager/local_api/network_mode.rs b/src/rust/lqosd/src/node_manager/local_api/network_mode.rs index dee2ce53..fa0a7af1 100644 --- a/src/rust/lqosd/src/node_manager/local_api/network_mode.rs +++ b/src/rust/lqosd/src/node_manager/local_api/network_mode.rs @@ -110,7 +110,14 @@ enum MergeNetworkModeError { fn merge_network_mode(candidate: Config) -> Result { let live = lqos_config::load_config().map_err(|_| MergeNetworkModeError::LoadLiveConfig)?; - let mut merged = (*live).clone(); + merge_network_mode_with_live(&live, candidate) +} + +fn merge_network_mode_with_live( + live: &Config, + candidate: Config, +) -> Result { + let mut merged = live.clone(); merged.bridge = candidate.bridge; merged.single_interface = candidate.single_interface; merged @@ -202,11 +209,9 @@ pub async fn apply( MergeNetworkModeError::InvalidCandidate(message) => { helper_validation_error_response(message) } - MergeNetworkModeError::LoadLiveConfig => { - helper_internal_error_response( - "Unable to load the live LibreQoS configuration".to_string(), - ) - } + MergeNetworkModeError::LoadLiveConfig => helper_internal_error_response( + "Unable to load the live LibreQoS configuration".to_string(), + ), }; } }; @@ -350,86 +355,8 @@ pub async fn retry_shaping( #[cfg(test)] mod tests { - use super::{MergeNetworkModeError, merge_network_mode}; - use crate::test_support::runtime_config_test_lock; + use super::{MergeNetworkModeError, merge_network_mode_with_live}; use lqos_config::{BridgeConfig, Config, SingleInterfaceConfig}; - use std::ffi::OsString; - use std::path::{Path, PathBuf}; - use std::time::{SystemTime, UNIX_EPOCH}; - - fn network_mode_test_runtime_dir() -> PathBuf { - let unique_suffix = SystemTime::now() - .duration_since(UNIX_EPOCH) - .expect("time should move forward") - .as_nanos(); - std::env::temp_dir().join(format!( - "lqosd-network-mode-test-{}-{unique_suffix}", - std::process::id() - )) - } - - fn write_network_mode_test_config(runtime_dir: &Path) -> PathBuf { - std::fs::create_dir_all(runtime_dir).expect("create network-mode runtime dir"); - let config_path = runtime_dir.join("lqos.conf"); - let runtime_dir_string = runtime_dir.display().to_string(); - let state_dir_string = runtime_dir.join("state").display().to_string(); - let raw = include_str!("../../../../lqos_config/src/etc/v15/example.toml") - .replace("/opt/libreqos/src", &runtime_dir_string) - .replace("/opt/libreqos/state", &state_dir_string) - .replace("node_id = \"0000-0000-0000\"", "node_id = \"node\""); - std::fs::write(&config_path, raw).expect("write network-mode test config"); - config_path - } - - struct NetworkModeTestContext { - _guard: std::sync::MutexGuard<'static, ()>, - old_lqos_config: Option, - old_lqos_directory: Option, - runtime_dir: PathBuf, - } - - impl NetworkModeTestContext { - fn new() -> Self { - let guard = runtime_config_test_lock() - .lock() - .expect("network-mode env lock should not be poisoned"); - let runtime_dir = network_mode_test_runtime_dir(); - let config_path = write_network_mode_test_config(&runtime_dir); - let old_lqos_config = std::env::var_os("LQOS_CONFIG"); - let old_lqos_directory = std::env::var_os("LQOS_DIRECTORY"); - unsafe { - std::env::set_var("LQOS_CONFIG", &config_path); - std::env::set_var("LQOS_DIRECTORY", &runtime_dir); - } - lqos_config::clear_cached_config(); - Self { - _guard: guard, - old_lqos_config, - old_lqos_directory, - runtime_dir, - } - } - } - - impl Drop for NetworkModeTestContext { - fn drop(&mut self) { - match &self.old_lqos_config { - Some(value) => unsafe { std::env::set_var("LQOS_CONFIG", value) }, - None => unsafe { std::env::remove_var("LQOS_CONFIG") }, - } - match &self.old_lqos_directory { - Some(value) => unsafe { std::env::set_var("LQOS_DIRECTORY", value) }, - None => unsafe { std::env::remove_var("LQOS_DIRECTORY") }, - } - lqos_config::clear_cached_config(); - let _ = std::fs::remove_dir_all(&self.runtime_dir); - } - } - - fn with_config_env(test_fn: impl FnOnce() -> T) -> T { - let _context = NetworkModeTestContext::new(); - test_fn() - } #[test] fn merge_network_mode_rejects_invalid_bridge_mtu() { @@ -439,8 +366,8 @@ mod tests { ..BridgeConfig::default() }); - let error = - with_config_env(|| merge_network_mode(candidate).expect_err("invalid MTU should fail")); + let error = merge_network_mode_with_live(&Config::default(), candidate) + .expect_err("invalid MTU should fail"); assert_eq!( error, @@ -459,8 +386,8 @@ mod tests { ..SingleInterfaceConfig::default() }); - let error = - with_config_env(|| merge_network_mode(candidate).expect_err("invalid MTU should fail")); + let error = merge_network_mode_with_live(&Config::default(), candidate) + .expect_err("invalid MTU should fail"); assert_eq!( error, diff --git a/src/rust/lqosd/src/node_manager/local_api/packet_analysis.rs b/src/rust/lqosd/src/node_manager/local_api/packet_analysis.rs index b89bcc89..2bfeb9ea 100644 --- a/src/rust/lqosd/src/node_manager/local_api/packet_analysis.rs +++ b/src/rust/lqosd/src/node_manager/local_api/packet_analysis.rs @@ -1,11 +1,12 @@ use axum::body::Body; use axum::extract::Path; -use axum::http::{HeaderMap, Request}; +use axum::http::{HeaderMap, Request, StatusCode}; use axum::response::IntoResponse; use lqos_heimdall::n_second_pcap; use serde::Serialize; use std::net::IpAddr; use tower_http::services::ServeFile; +use tracing::warn; #[derive(Debug, Serialize, Clone)] pub enum RequestAnalysisResult { @@ -25,12 +26,35 @@ pub fn request_analysis_data(ip: &str) -> RequestAnalysisResult { RequestAnalysisResult::Fail } -pub async fn pcap_dump(Path(id): Path, headers: HeaderMap) -> impl IntoResponse { - let filename = n_second_pcap(id).expect("Could not determine pcap filename"); +pub async fn pcap_dump( + Path(id): Path, + headers: HeaderMap, +) -> Result { + let Some(filename) = n_second_pcap(id) else { + return Err(StatusCode::NOT_FOUND); + }; + let mut req = Request::new(Body::empty()); *req.headers_mut() = headers; - ServeFile::new(filename) - .try_call(req) - .await - .expect("ServeFile call failed") + match ServeFile::new(&filename).try_call(req).await { + Ok(response) => Ok(response), + Err(err) => { + warn!("Unable to serve packet capture file {filename}: {err}"); + Err(StatusCode::INTERNAL_SERVER_ERROR) + } + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[tokio::test] + async fn pcap_dump_returns_not_found_for_missing_session() { + let response = pcap_dump(Path(usize::MAX), HeaderMap::new()) + .await + .into_response(); + + assert_eq!(response.status(), StatusCode::NOT_FOUND); + } } diff --git a/src/rust/lqosd/src/node_manager/run.rs b/src/rust/lqosd/src/node_manager/run.rs index 1c049a7d..9f90aab1 100644 --- a/src/rust/lqosd/src/node_manager/run.rs +++ b/src/rust/lqosd/src/node_manager/run.rs @@ -10,7 +10,7 @@ use crate::system_stats::SystemStats; use anyhow::{Result, bail}; use axum::Router; use axum::http::StatusCode; -use axum::response::Redirect; +use axum::response::{IntoResponse, Redirect, Response}; use axum::routing::{get, post}; use lqos_bus::BusRequest; use lqos_config::load_config; @@ -18,7 +18,6 @@ use lqos_probe::ProbeClient; use std::path::Path; use tokio::net::TcpListener; use tokio::sync::mpsc::Sender; -use tower_http::cors::CorsLayer; use tower_http::services::{ServeDir, ServeFile}; use tracing::info; @@ -59,6 +58,7 @@ pub async fn spawn_webserver( .route("/doLogin", post(auth::try_login)) .route("/firstLogin", post(auth::first_user)) .route("/health", get(health_check)) + .route("/template.html", get(not_found)) .route("/configuration.html", get(redirect_configuration_page)) // Backwards compatible aliases for historical misspellings. .route_service( @@ -82,11 +82,14 @@ pub async fn spawn_webserver( .nest("/vendor", vendor_route()?) // Serve /vendor as purely static .nest("/", static_routes()?) .nest("/local-api", local_api(shaper_tx)) - .fallback_service(ServeDir::new(static_path)) - .layer(CorsLayer::very_permissive()); + .fallback_service(ServeDir::new(static_path)); info!("Webserver listening on: [{listen_address}]"); - axum::serve(listener, router).await?; + axum::serve( + listener, + router.into_make_service_with_connect_info::(), + ) + .await?; Ok(()) } @@ -104,3 +107,23 @@ async fn redirect_configuration_page() -> Redirect { async fn health_check() -> (StatusCode, &'static str) { (StatusCode::OK, "OK") } + +async fn not_found() -> Response { + StatusCode::NOT_FOUND.into_response() +} + +#[cfg(test)] +mod tests { + #[test] + fn node_manager_router_sources_do_not_enable_cors_layer() { + let sources = [ + include_str!("run.rs"), + include_str!("local_api.rs"), + include_str!("ws.rs"), + ]; + for source in sources { + assert!(!source.contains(concat!("Cors", "Layer"))); + assert!(!source.contains(concat!("tower_http::", "cors"))); + } + } +} diff --git a/src/rust/lqosd/src/node_manager/security_headers.rs b/src/rust/lqosd/src/node_manager/security_headers.rs new file mode 100644 index 00000000..c84df93b --- /dev/null +++ b/src/rust/lqosd/src/node_manager/security_headers.rs @@ -0,0 +1,58 @@ +use axum::http::{HeaderMap, HeaderName, HeaderValue}; + +// The current templates still use small inline boot scripts and inline layout styles. +// Keep script/style sources otherwise self-hosted so future remote script drift is visible. +pub(crate) const NODE_MANAGER_CONTENT_SECURITY_POLICY: &str = concat!( + "default-src 'self'; ", + "base-uri 'self'; ", + "object-src 'none'; ", + "form-action 'self'; ", + "frame-ancestors 'none'; ", + "script-src 'self' 'unsafe-inline'; ", + "style-src 'self' 'unsafe-inline'; ", + "img-src 'self' data: blob: https://insight.libreqos.com; ", + "font-src 'self' data:; ", + "connect-src 'self' ws: wss: https://insight.libreqos.com; ", + "frame-src 'self' about: http://*:9122; ", + "worker-src 'self' blob:" +); + +pub(crate) fn apply_node_manager_security_headers(headers: &mut HeaderMap) { + headers.insert( + HeaderName::from_static("content-security-policy"), + HeaderValue::from_static(NODE_MANAGER_CONTENT_SECURITY_POLICY), + ); +} + +#[cfg(test)] +mod tests { + use super::{NODE_MANAGER_CONTENT_SECURITY_POLICY, apply_node_manager_security_headers}; + use axum::http::{HeaderMap, HeaderName}; + + #[test] + fn node_manager_csp_matches_current_ui_requirements() { + let csp = NODE_MANAGER_CONTENT_SECURITY_POLICY; + + assert!(csp.contains("default-src 'self'")); + assert!(csp.contains("script-src 'self' 'unsafe-inline'")); + assert!(csp.contains("style-src 'self' 'unsafe-inline'")); + assert!(csp.contains("connect-src 'self' ws: wss: https://insight.libreqos.com")); + assert!(csp.contains("img-src 'self' data: blob: https://insight.libreqos.com")); + assert!(csp.contains("frame-src 'self' about: http://*:9122")); + assert!(csp.contains("frame-ancestors 'none'")); + assert!(!csp.contains("https://fastly.jsdelivr.net")); + } + + #[test] + fn applies_content_security_policy_header() { + let mut headers = HeaderMap::new(); + + apply_node_manager_security_headers(&mut headers); + + let header_name = HeaderName::from_static("content-security-policy"); + assert_eq!( + headers.get(header_name), + Some(&NODE_MANAGER_CONTENT_SECURITY_POLICY.parse().unwrap()) + ); + } +} diff --git a/src/rust/lqosd/src/node_manager/static2/first-run.html b/src/rust/lqosd/src/node_manager/static2/first-run.html index 7f4c89f2..3ca744f9 100644 --- a/src/rust/lqosd/src/node_manager/static2/first-run.html +++ b/src/rust/lqosd/src/node_manager/static2/first-run.html @@ -42,23 +42,16 @@ This site will use a cookie to store your identification. If that's not ok, please don't use the site.

-

Let's create a new user, and set some parameters:

+

Let's create a new admin user:

- - - - - + + diff --git a/src/rust/lqosd/src/node_manager/static_pages.rs b/src/rust/lqosd/src/node_manager/static_pages.rs index 5ea24fe0..a57108a5 100644 --- a/src/rust/lqosd/src/node_manager/static_pages.rs +++ b/src/rust/lqosd/src/node_manager/static_pages.rs @@ -4,7 +4,6 @@ use anyhow::{Result, bail}; use axum::Router; use lqos_config::load_config; use std::path::Path; -use tower_http::cors::CorsLayer; use tower_http::services::{ServeDir, ServeFile}; pub(super) fn vendor_route() -> Result { @@ -112,7 +111,6 @@ pub(super) fn static_routes() -> Result { router = router.route_service("/config_spylnx.html", ServeFile::new(splynx_path)); router = router - .layer(CorsLayer::very_permissive()) .route_layer(axum::middleware::from_fn(auth_layer)) .route_layer(axum::middleware::from_fn(apply_templates)); diff --git a/src/rust/lqosd/src/node_manager/template.rs b/src/rust/lqosd/src/node_manager/template.rs index 774a0db9..e5f4e641 100644 --- a/src/rust/lqosd/src/node_manager/template.rs +++ b/src/rust/lqosd/src/node_manager/template.rs @@ -2,6 +2,7 @@ //! files. use crate::node_manager::auth::get_username; +use crate::node_manager::security_headers::apply_node_manager_security_headers; use crate::tool_status::is_api_available; use axum::body::{Body, to_bytes}; use axum::http::header; @@ -254,6 +255,7 @@ pub async fn apply_templates( res_parts .headers .insert(header::CACHE_CONTROL, HeaderValue::from_static("no-store")); + apply_node_manager_security_headers(&mut res_parts.headers); let res = Response::from_parts(res_parts, Body::from(byte_string)); Ok(res) } else { diff --git a/src/rust/lqosd/src/node_manager/ws.rs b/src/rust/lqosd/src/node_manager/ws.rs index 867b4e91..da6c7ab1 100644 --- a/src/rust/lqosd/src/node_manager/ws.rs +++ b/src/rust/lqosd/src/node_manager/ws.rs @@ -10,7 +10,7 @@ use std::sync::Arc; use std::time::Duration; use crate::lts2_sys::control_channel::ControlChannelCommand; -use crate::node_manager::auth::{LoginResult, login_from_token}; +use crate::node_manager::auth::{LoginResult, login_from_cookie_header}; use crate::node_manager::local_api::{ circuit, circuit_count, config, cpu_affinity, dashboard_themes, device_counts, directories, ethernet_caps, executive, flow_explorer, flow_map, lts, network_tree, network_tree_lite, @@ -38,8 +38,8 @@ use axum::{ WebSocketUpgrade, ws::{Message, WebSocket}, }, - http::{HeaderMap, StatusCode, header}, - response::IntoResponse, + http::{HeaderMap, StatusCode, Uri, header}, + response::{IntoResponse, Response}, routing::get, }; use futures_util::{SinkExt, StreamExt}; @@ -66,6 +66,16 @@ static TOPOLOGY_MANAGER_READ_PERMITS: Lazy> = static TOPOLOGY_MANAGER_BLOCKING_PERMITS: Lazy> = Lazy::new(|| Arc::new(Semaphore::new(1))); +struct WsUpgradeContext { + channels: Arc, + bus_tx: Sender<(tokio::sync::oneshot::Sender, BusRequest)>, + control_tx: tokio::sync::mpsc::Sender, + probe_client: ProbeClient, + shaper_query: Sender, + browser_language: Option, + login: LoginResult, +} + async fn send_control_command( control_tx: &tokio::sync::mpsc::Sender, command: ControlChannelCommand, @@ -193,7 +203,12 @@ async fn ws_handler( Extension(probe_client): Extension, Extension(shaper_query): Extension>, headers: HeaderMap, -) -> impl IntoResponse { +) -> Response { + if !websocket_origin_allowed(&headers) { + warn!("Rejected websocket upgrade with cross-origin Origin header"); + return StatusCode::FORBIDDEN.into_response(); + } + let has_cookie = headers.contains_key(header::COOKIE); let user_agent = headers .get(header::USER_AGENT) @@ -205,31 +220,75 @@ async fn ws_handler( .get(header::ACCEPT_LANGUAGE) .and_then(|h| h.to_str().ok()) .map(|s| s.to_string()); + let login = login_from_cookie_header( + headers + .get(header::COOKIE) + .and_then(|value| value.to_str().ok()), + ) + .await; ws.on_upgrade(move |socket| async move { handle_socket( socket, - channels, - bus_tx, - control_tx, - probe_client, - shaper_query, - browser_language, + WsUpgradeContext { + channels, + bus_tx, + control_tx, + probe_client, + shaper_query, + browser_language, + login, + }, ) .await; }) + .into_response() } -async fn handle_socket( - socket: WebSocket, - channels: Arc, - bus_tx: Sender<(tokio::sync::oneshot::Sender, BusRequest)>, - control_tx: tokio::sync::mpsc::Sender, - probe_client: ProbeClient, - shaper_query: Sender, - browser_language: Option, -) { +fn websocket_origin_allowed(headers: &HeaderMap) -> bool { + let Some(origin) = headers + .get(header::ORIGIN) + .and_then(|value| value.to_str().ok()) + .map(str::trim) + else { + return true; + }; + + let Some(host) = headers + .get(header::HOST) + .and_then(|value| value.to_str().ok()) + .map(str::trim) + else { + return false; + }; + + let Ok(uri) = origin.parse::() else { + return false; + }; + let Some(scheme) = uri.scheme_str() else { + return false; + }; + if scheme != "http" && scheme != "https" { + return false; + } + + uri.authority() + .map(|authority| authority.as_str().eq_ignore_ascii_case(host)) + .unwrap_or(false) +} + +async fn handle_socket(socket: WebSocket, context: WsUpgradeContext) { info!("Websocket connected"); + let WsUpgradeContext { + channels, + bus_tx, + control_tx, + probe_client, + shaper_query, + browser_language, + mut login, + } = context; + let (mut ws_tx, mut ws_rx) = socket.split(); // Larger buffer helps absorb bursts of pubsub updates without stalling @@ -244,7 +303,6 @@ async fn handle_socket( }); let mut subscribed_channels = HashSet::new(); let mut handshake_complete = false; - let mut login = LoginResult::Denied; let handshake_timeout = tokio::time::sleep(std::time::Duration::from_secs(HANDSHAKE_TIMEOUT_SECS)); tokio::pin!(handshake_timeout); @@ -342,6 +400,10 @@ struct WsRequestState<'a> { shaper_query: Sender, } +fn can_write_dashboard_themes(login: LoginResult) -> bool { + login == LoginResult::Admin +} + async fn receive_channel_message( msg: Message, channels: Arc, @@ -392,13 +454,10 @@ async fn receive_channel_message( warn!("Websocket handshake ack mismatch"); return true; } - let token = reply.token.trim(); - let login_result = login_from_token(token).await; - if login_result == LoginResult::Denied { - warn!("Websocket handshake token rejected"); + if *request_state.login == LoginResult::Denied { + warn!("Websocket handshake cookie rejected"); return true; } - *request_state.login = login_result; *handshake_complete = true; info!("Websocket handshake completed"); return false; @@ -430,17 +489,25 @@ async fn receive_channel_message( } } WsRequest::DashletSave { name, entries } => { - let data = dashboard_themes::DashletSave { name, entries }; - let result = dashboard_themes::save_theme_data(&data); - let response = match result { - Ok(_) => WsResponse::DashletSaveResult { - ok: true, - error: None, - }, - Err(err) => WsResponse::DashletSaveResult { + let response = if can_write_dashboard_themes(*request_state.login) { + let data = dashboard_themes::DashletSave { name, entries }; + match dashboard_themes::save_theme_data(&data) { + Ok(_) => WsResponse::DashletSaveResult { + ok: true, + error: None, + }, + Err(err) => WsResponse::DashletSaveResult { + ok: false, + error: Some(err), + }, + } + } else { + WsResponse::DashletSaveResult { ok: false, - error: Some(err), - }, + error: Some( + "Only administrators can save shared dashboard layouts.".to_string(), + ), + } }; if send_ws_response(&tx, response).await { return true; @@ -454,16 +521,24 @@ async fn receive_channel_message( } } WsRequest::DashletDelete { name } => { - let result = dashboard_themes::delete_theme_file(&name); - let response = match result { - Ok(_) => WsResponse::DashletDeleteResult { - ok: true, - error: None, - }, - Err(err) => WsResponse::DashletDeleteResult { + let response = if can_write_dashboard_themes(*request_state.login) { + match dashboard_themes::delete_theme_file(&name) { + Ok(_) => WsResponse::DashletDeleteResult { + ok: true, + error: None, + }, + Err(err) => WsResponse::DashletDeleteResult { + ok: false, + error: Some(err), + }, + } + } else { + WsResponse::DashletDeleteResult { ok: false, - error: Some(err), - }, + error: Some( + "Only administrators can delete shared dashboard layouts.".to_string(), + ), + } }; if send_ws_response(&tx, response).await { return true; @@ -738,29 +813,31 @@ async fn receive_channel_message( } } WsRequest::AsnFlowTimeline { asn } => { - let response = WsResponse::AsnFlowTimeline { - asn, - data: flow_explorer::flow_timeline_data(asn), - }; - if send_ws_response(&tx, response).await { + let response = flow_explorer::flow_timeline_data(asn) + .map(|data| WsResponse::AsnFlowTimeline { asn, data }); + if send_flow_timeline_response(&tx, "ASN", response).await { return true; } } WsRequest::CountryFlowTimeline { iso_code } => { - let response = WsResponse::CountryFlowTimeline { - iso_code: iso_code.clone(), - data: flow_explorer::country_timeline_data(&iso_code), - }; - if send_ws_response(&tx, response).await { + let response = flow_explorer::country_timeline_data(&iso_code).map(|data| { + WsResponse::CountryFlowTimeline { + iso_code: iso_code.clone(), + data, + } + }); + if send_flow_timeline_response(&tx, "country", response).await { return true; } } WsRequest::ProtocolFlowTimeline { protocol } => { - let response = WsResponse::ProtocolFlowTimeline { - protocol: protocol.clone(), - data: flow_explorer::protocol_timeline_data(&protocol), - }; - if send_ws_response(&tx, response).await { + let response = flow_explorer::protocol_timeline_data(&protocol).map(|data| { + WsResponse::ProtocolFlowTimeline { + protocol: protocol.clone(), + data, + } + }); + if send_flow_timeline_response(&tx, "protocol", response).await { return true; } } @@ -2596,6 +2673,26 @@ fn maybe_schedule_lts_signup_shutdown( } } +async fn send_flow_timeline_response( + tx: &Sender>>, + label: &str, + response: Result, +) -> bool { + match response { + Ok(response) => send_ws_response(tx, response).await, + Err(err) => { + warn!("Unable to build {label} flow timeline: {err}"); + send_ws_response( + tx, + WsResponse::Error { + message: format!("Unable to build {label} flow timeline"), + }, + ) + .await + } + } +} + fn decode_ws_request(payload: &[u8]) -> Result { let prefix = payload_prefix_hex(payload, 24); let hint = payload_hint(payload); @@ -2652,13 +2749,15 @@ fn payload_hint(payload: &[u8]) -> &'static str { #[cfg(test)] mod tests { use super::{ - decode_ws_request, maybe_schedule_lts_signup_shutdown, + can_write_dashboard_themes, decode_ws_request, maybe_schedule_lts_signup_shutdown, run_topology_manager_blocking_with_timeout, run_topology_manager_mutation_blocking, + websocket_origin_allowed, }; + use crate::node_manager::auth::LoginResult; use crate::node_manager::local_api::urgent::{UrgentList, UrgentStatus}; use crate::node_manager::ws::messages::WsRequest; use crate::node_manager::ws::messages::{WsResponse, encode_ws_message}; - use axum::http::StatusCode; + use axum::http::{HeaderMap, HeaderValue, StatusCode, header}; use serde_cbor::Value as CborValue; use std::collections::BTreeMap; use std::sync::{ @@ -2917,6 +3016,53 @@ mod tests { entries.contains_key(&text("request_id")) } + fn origin_headers(host: &str, origin: Option<&str>) -> HeaderMap { + let mut headers = HeaderMap::new(); + headers.insert(header::HOST, HeaderValue::from_str(host).unwrap()); + if let Some(origin) = origin { + headers.insert(header::ORIGIN, HeaderValue::from_str(origin).unwrap()); + } + headers + } + + #[test] + fn read_only_websocket_users_cannot_write_dashboard_themes() { + assert!(can_write_dashboard_themes(LoginResult::Admin)); + assert!(!can_write_dashboard_themes(LoginResult::ReadOnly)); + assert!(!can_write_dashboard_themes(LoginResult::Denied)); + } + + #[test] + fn websocket_origin_allows_same_origin_and_non_browser_clients() { + assert!(websocket_origin_allowed(&origin_headers( + "node.example.test:9123", + Some("https://node.example.test:9123") + ))); + assert!(websocket_origin_allowed(&origin_headers( + "node.example.test:9123", + None + ))); + } + + #[test] + fn websocket_origin_rejects_cross_origin_and_invalid_origin() { + assert!(!websocket_origin_allowed(&origin_headers( + "node.example.test:9123", + Some("https://evil.example.test") + ))); + assert!(!websocket_origin_allowed(&origin_headers( + "node.example.test:9123", + Some("null") + ))); + + let mut headers = HeaderMap::new(); + headers.insert( + header::ORIGIN, + HeaderValue::from_static("https://node.example.test:9123"), + ); + assert!(!websocket_origin_allowed(&headers)); + } + #[test] fn decodes_urgent_status_with_request_id() { let payload = serde_cbor::to_vec(&map(vec![( diff --git a/src/rust/lqosd/src/throughput_tracker/flow_data/mod.rs b/src/rust/lqosd/src/throughput_tracker/flow_data/mod.rs index c7fea529..b90a962f 100644 --- a/src/rust/lqosd/src/throughput_tracker/flow_data/mod.rs +++ b/src/rust/lqosd/src/throughput_tracker/flow_data/mod.rs @@ -6,6 +6,7 @@ mod flow_analysis; mod flow_tracker; mod netflow5; mod netflow9; +mod netflow_common; use crate::throughput_tracker::flow_data::{ flow_analysis::FinishedFlowAnalysis, netflow5::Netflow5, netflow9::Netflow9, diff --git a/src/rust/lqosd/src/throughput_tracker/flow_data/netflow5/mod.rs b/src/rust/lqosd/src/throughput_tracker/flow_data/netflow5/mod.rs index a2ffe9d9..e783c11b 100644 --- a/src/rust/lqosd/src/throughput_tracker/flow_data/netflow5/mod.rs +++ b/src/rust/lqosd/src/throughput_tracker/flow_data/netflow5/mod.rs @@ -29,7 +29,7 @@ impl Netflow5 { }; let sequence = AtomicU32::new(0); - let mut accumulator = Vec::with_capacity(15); + let mut accumulator = Vec::with_capacity(NETFLOW5_MAX_FLOWS_PER_PACKET); let mut last_sent = std::time::Instant::now(); while let Ok((key, (data, analysis))) = rx.recv() { // Exclude one-way flows @@ -39,11 +39,10 @@ impl Netflow5 { accumulator.push((key, (data, analysis))); - // Send if there is more than 15 records AND it has been more than 1 second since the last send - if accumulator.len() >= 15 && last_sent.elapsed().as_secs() > 1 { - for chunk in accumulator.chunks(15) { - Self::queue_handler(chunk, &socket, &target, &sequence); - } + if accumulator.len() >= NETFLOW5_MAX_FLOWS_PER_PACKET + || last_sent.elapsed().as_secs() > 1 + { + Self::flush_accumulator(&accumulator, &socket, &target, &sequence); accumulator.clear(); last_sent = std::time::Instant::now(); } @@ -51,22 +50,58 @@ impl Netflow5 { // Handle any remaining flows when shutting down if !accumulator.is_empty() { - for chunk in accumulator.chunks(15) { - Self::queue_handler(chunk, &socket, &target, &sequence); - } + Self::flush_accumulator(&accumulator, &socket, &target, &sequence); } })?; Ok(tx) } + fn flush_accumulator( + accumulator: &[(FlowbeeKey, (FlowbeeLocalData, FlowAnalysis))], + socket: &UdpSocket, + target: &str, + sequence: &AtomicU32, + ) { + for chunk in accumulator.chunks(NETFLOW5_MAX_FLOWS_PER_PACKET) { + Self::queue_handler(chunk, socket, target, sequence); + } + } + fn queue_handler( accumulator: &[(FlowbeeKey, (FlowbeeLocalData, FlowAnalysis))], socket: &UdpSocket, target: &str, sequence: &AtomicU32, ) { - let num_records = (accumulator.len() * 2) as u16; + if accumulator.is_empty() { + return; + } + + if accumulator.len() > NETFLOW5_MAX_FLOWS_PER_PACKET { + for chunk in accumulator.chunks(NETFLOW5_MAX_FLOWS_PER_PACKET) { + Self::queue_handler(chunk, socket, target, sequence); + } + return; + } + + let mut records = Vec::with_capacity(accumulator.len() * 2); + for (key, (data, _)) in accumulator { + if let Ok((packet1, packet2)) = to_netflow_5(key, data) { + records.push(packet1); + records.push(packet2); + } + } + + let Ok(num_records) = u16::try_from(records.len()) else { + tracing::error!("NetFlow5 record count exceeded u16::MAX; dropping export packet"); + return; + }; + + if num_records == 0 { + return; + } + let sequence_number = sequence.load(std::sync::atomic::Ordering::Relaxed); let header = Netflow5Header::new(sequence_number, num_records); let header_bytes = unsafe { @@ -77,34 +112,25 @@ impl Netflow5 { }; let mut buffer = Vec::with_capacity( - header_bytes.len() + (accumulator.len() * 2 * std::mem::size_of::()), + header_bytes.len() + (records.len() * std::mem::size_of::()), ); buffer.extend_from_slice(header_bytes); - for (key, (data, _)) in accumulator { - if let Ok((packet1, packet2)) = to_netflow_5(key, data) { - let packet1_bytes = unsafe { - std::slice::from_raw_parts( - &packet1 as *const _ as *const u8, - std::mem::size_of::(), - ) - }; - let packet2_bytes = unsafe { - std::slice::from_raw_parts( - &packet2 as *const _ as *const u8, - std::mem::size_of::(), - ) - }; - buffer.extend_from_slice(packet1_bytes); - buffer.extend_from_slice(packet2_bytes); - } + for record in &records { + let record_bytes = unsafe { + std::slice::from_raw_parts( + record as *const _ as *const u8, + std::mem::size_of::(), + ) + }; + buffer.extend_from_slice(record_bytes); } if let Err(e) = socket.send_to(&buffer, target) { tracing::error!("Failed to send Netflow5 data to {}: {}", target, e); // Don't increment sequence on failure to maintain consistency } else { - sequence.fetch_add(num_records as u32, std::sync::atomic::Ordering::Relaxed); + sequence.fetch_add(u32::from(num_records), std::sync::atomic::Ordering::Relaxed); } } } diff --git a/src/rust/lqosd/src/throughput_tracker/flow_data/netflow5/protocol.rs b/src/rust/lqosd/src/throughput_tracker/flow_data/netflow5/protocol.rs index 3c8dfbc7..1663fb46 100644 --- a/src/rust/lqosd/src/throughput_tracker/flow_data/netflow5/protocol.rs +++ b/src/rust/lqosd/src/throughput_tracker/flow_data/netflow5/protocol.rs @@ -6,6 +6,12 @@ use nix::sys::time::TimeValLike; use std::net::IpAddr; use crate::throughput_tracker::flow_data::FlowbeeLocalData; +use crate::throughput_tracker::flow_data::netflow_common::{ + boot_nanos_to_netflow_millis, clamp_i64_to_u32, clamp_u64_to_u32, +}; + +const NETFLOW5_MAX_RECORDS_PER_PACKET: usize = 30; +pub(crate) const NETFLOW5_MAX_FLOWS_PER_PACKET: usize = NETFLOW5_MAX_RECORDS_PER_PACKET / 2; /// Standard Netflow 5 header #[repr(C)] @@ -25,7 +31,7 @@ impl Netflow5Header { /// Create a new Netflow 5 header pub(crate) fn new(flow_sequence: u32, num_records: u16) -> Self { let uptime_ms: u32 = time_since_boot() - .map(|u| u.num_milliseconds() as u32) + .map(|u| clamp_i64_to_u32("NetFlow5", "sys_uptime", u.num_milliseconds())) .unwrap_or(0); let unix_secs = unix_now().unwrap_or(0); @@ -33,7 +39,7 @@ impl Netflow5Header { version: (5u16).to_be(), count: num_records.to_be(), sys_uptime: uptime_ms.to_be(), - unix_secs: (unix_secs as u32).to_be(), + unix_secs: clamp_u64_to_u32("NetFlow5", "unix_secs", unix_secs).to_be(), unix_nsecs: 0, flow_sequence, engine_type: 0, @@ -73,17 +79,17 @@ pub(crate) fn to_netflow_5( key: &FlowbeeKey, data: &FlowbeeLocalData, ) -> anyhow::Result<(Netflow5Record, Netflow5Record)> { - // TODO: Detect overflow let local = key.local_ip.as_ip(); let remote = key.remote_ip.as_ip(); if let (IpAddr::V4(local), IpAddr::V4(remote)) = (local, remote) { let src_ip = u32::from_ne_bytes(local.octets()); let dst_ip = u32::from_ne_bytes(remote.octets()); - // Convert d_pkts to network order - let d_pkts2 = (data.packets_sent.down as u32).to_be(); - let d_octets2 = (data.bytes_sent.down as u32).to_be(); - let d_pkts = (data.packets_sent.up as u32).to_be(); - let d_octets = (data.bytes_sent.up as u32).to_be(); + let d_pkts2 = clamp_u64_to_u32("NetFlow5", "down packets", data.packets_sent.down).to_be(); + let d_octets2 = clamp_u64_to_u32("NetFlow5", "down octets", data.bytes_sent.down).to_be(); + let d_pkts = clamp_u64_to_u32("NetFlow5", "up packets", data.packets_sent.up).to_be(); + let d_octets = clamp_u64_to_u32("NetFlow5", "up octets", data.bytes_sent.up).to_be(); + let first = boot_nanos_to_netflow_millis("NetFlow5", "first", data.start_time).to_be(); + let last = boot_nanos_to_netflow_millis("NetFlow5", "last", data.last_seen).to_be(); let record = Netflow5Record { src_addr: src_ip, @@ -93,8 +99,8 @@ pub(crate) fn to_netflow_5( output: (1u16).to_be(), d_pkts, d_octets, - first: ((data.start_time / 1_000_000) as u32).to_be(), // Convert to milliseconds - last: ((data.last_seen / 1_000_000) as u32).to_be(), // Convert to milliseconds + first, + last, src_port: key.src_port.to_be(), dst_port: key.dst_port.to_be(), pad1: 0, @@ -116,8 +122,8 @@ pub(crate) fn to_netflow_5( output: 0, d_pkts: d_pkts2, d_octets: d_octets2, - first: data.start_time as u32, // Convert to milliseconds - last: data.last_seen as u32, // Convert to milliseconds + first, + last, src_port: key.dst_port.to_be(), dst_port: key.src_port.to_be(), pad1: 0, @@ -136,3 +142,69 @@ pub(crate) fn to_netflow_5( Err(anyhow::anyhow!("Only IPv4 is supported")) } } + +#[cfg(test)] +mod tests { + use super::*; + use crate::throughput_tracker::flow_data::FlowbeeLocalData; + use crate::throughput_tracker::flow_data::netflow_common::NANOS_PER_MILLI; + use lqos_utils::{XdpIpAddress, units::DownUpOrder}; + use std::net::IpAddr; + + fn test_key() -> FlowbeeKey { + let mut key = FlowbeeKey::default(); + key.local_ip = XdpIpAddress::from_ip(IpAddr::from([192, 0, 2, 10])); + key.remote_ip = XdpIpAddress::from_ip(IpAddr::from([198, 51, 100, 20])); + key.src_port = 12345; + key.dst_port = 443; + key.ip_protocol = 6; + key + } + + fn test_flow_data() -> FlowbeeLocalData { + FlowbeeLocalData { + start_time: 1_500_000, + last_seen: 2_500_000, + bytes_sent: DownUpOrder::new(u64::from(u32::MAX) + 10, 20), + packets_sent: DownUpOrder::new(30, u64::from(u32::MAX) + 10), + rate_estimate_bps: DownUpOrder::new(0, 0), + display_rate_bps: None, + tcp_retransmits: DownUpOrder::new(0, 0), + end_status: 0, + tos: 0, + tc_handle: 0, + cpu: 0, + circuit_hash: None, + device_hash: None, + tcp_info: None, + } + } + + #[test] + fn netflow5_records_clamp_counters_and_use_milliseconds() { + let key = test_key(); + let data = test_flow_data(); + + let (forward, reverse) = + to_netflow_5(&key, &data).expect("IPv4 flow should convert to NetFlow5 records"); + + assert_eq!(u32::from_be(forward.d_pkts), u32::MAX); + assert_eq!(u32::from_be(forward.d_octets), 20); + assert_eq!(u32::from_be(reverse.d_pkts), 30); + assert_eq!(u32::from_be(reverse.d_octets), u32::MAX); + assert_eq!(u32::from_be(forward.first), 1); + assert_eq!(u32::from_be(forward.last), 2); + assert_eq!(u32::from_be(reverse.first), 1); + assert_eq!(u32::from_be(reverse.last), 2); + } + + #[test] + fn netflow5_timestamp_conversion_clamps_after_milliseconds() { + let too_many_millis = (u64::from(u32::MAX) + 1) * NANOS_PER_MILLI; + + assert_eq!( + boot_nanos_to_netflow_millis("NetFlow5", "test timestamp", too_many_millis), + u32::MAX + ); + } +} diff --git a/src/rust/lqosd/src/throughput_tracker/flow_data/netflow9/protocol/header.rs b/src/rust/lqosd/src/throughput_tracker/flow_data/netflow9/protocol/header.rs index 704b493a..c0655dea 100644 --- a/src/rust/lqosd/src/throughput_tracker/flow_data/netflow9/protocol/header.rs +++ b/src/rust/lqosd/src/throughput_tracker/flow_data/netflow9/protocol/header.rs @@ -1,6 +1,8 @@ use lqos_utils::unix_time::{time_since_boot, unix_now}; use nix::sys::time::TimeValLike; +use crate::throughput_tracker::flow_data::netflow_common::{clamp_i64_to_u32, clamp_u64_to_u32}; + #[repr(C)] pub(crate) struct Netflow9Header { pub(crate) version: u16, @@ -14,18 +16,53 @@ pub(crate) struct Netflow9Header { impl Netflow9Header { /// Create a new Netflow 9 header pub(crate) fn new(flow_sequence: u32, record_count_including_templates: u16) -> Self { - let uptime_ms: u32 = time_since_boot() - .map(|u| u.num_milliseconds() as u32) - .unwrap_or(0); + let uptime_ms = time_since_boot().map(|u| u.num_milliseconds()).unwrap_or(0); let unix_secs = unix_now().unwrap_or(0); + Self::from_times( + flow_sequence, + record_count_including_templates, + uptime_ms, + unix_secs, + ) + } + + fn from_times( + flow_sequence: u32, + record_count_including_templates: u16, + uptime_ms: i64, + unix_secs: u64, + ) -> Self { Self { version: (9u16).to_be(), count: record_count_including_templates.to_be(), - sys_uptime: uptime_ms.to_be(), - unix_secs: (unix_secs as u32).to_be(), + sys_uptime: clamp_i64_to_u32("NetFlow9", "sys_uptime", uptime_ms).to_be(), + unix_secs: clamp_u64_to_u32("NetFlow9", "unix_secs", unix_secs).to_be(), package_sequence: flow_sequence.to_be(), source_id: 0, } } } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn netflow9_header_clamps_large_times() { + let header = + Netflow9Header::from_times(10, 2, i64::from(u32::MAX) + 1, u64::from(u32::MAX) + 1); + + assert_eq!(u32::from_be(header.sys_uptime), u32::MAX); + assert_eq!(u32::from_be(header.unix_secs), u32::MAX); + assert_eq!(u32::from_be(header.package_sequence), 10); + } + + #[test] + fn netflow9_header_clamps_negative_uptime_to_zero() { + let header = Netflow9Header::from_times(10, 2, -1, 1); + + assert_eq!(u32::from_be(header.sys_uptime), 0); + assert_eq!(u32::from_be(header.unix_secs), 1); + } +} diff --git a/src/rust/lqosd/src/throughput_tracker/flow_data/netflow_common.rs b/src/rust/lqosd/src/throughput_tracker/flow_data/netflow_common.rs new file mode 100644 index 00000000..c6669c88 --- /dev/null +++ b/src/rust/lqosd/src/throughput_tracker/flow_data/netflow_common.rs @@ -0,0 +1,28 @@ +//! Shared helpers for NetFlow wire-field conversions. + +pub(crate) const NANOS_PER_MILLI: u64 = 1_000_000; + +pub(crate) fn clamp_i64_to_u32(protocol: &str, field: &str, value: i64) -> u32 { + if value < 0 { + tracing::warn!("{protocol} {field} value {value} is negative; clamping to 0"); + return 0; + } + + clamp_u64_to_u32(protocol, field, value as u64) +} + +pub(crate) fn clamp_u64_to_u32(protocol: &str, field: &str, value: u64) -> u32 { + match u32::try_from(value) { + Ok(value) => value, + Err(_) => { + tracing::warn!( + "{protocol} {field} value {value} exceeds u32::MAX; clamping to u32::MAX" + ); + u32::MAX + } + } +} + +pub(crate) fn boot_nanos_to_netflow_millis(protocol: &str, field: &str, value: u64) -> u32 { + clamp_u64_to_u32(protocol, field, value / NANOS_PER_MILLI) +} diff --git a/src/rust/lqosd/src/throughput_tracker/stats_submission.rs b/src/rust/lqosd/src/throughput_tracker/stats_submission.rs index 0379be64..dcb94869 100644 --- a/src/rust/lqosd/src/throughput_tracker/stats_submission.rs +++ b/src/rust/lqosd/src/throughput_tracker/stats_submission.rs @@ -62,15 +62,60 @@ fn load_insight_logical_network_json() -> anyhow::Result { } /// Temporary conversion function for LTS/Insight compatibility -/// TODO: Remove when LTS/Insight support fractional rates fn rate_for_submission(rate_mbps: f32) -> u32 { + if rate_mbps.is_nan() { + warn!("LTS2 bandwidth value {rate_mbps} is not finite; clamping to 1 Mbps"); + return 1; + } + if rate_mbps.is_infinite() { + if rate_mbps.is_sign_positive() { + warn!("LTS2 bandwidth value {rate_mbps} exceeds u32::MAX; clamping to u32::MAX"); + return u32::MAX; + } + warn!("LTS2 bandwidth value {rate_mbps} is not finite; clamping to 1 Mbps"); + return 1; + } + if rate_mbps < 1.0 { 1 // Round up small fractional rates to 1 Mbps for now + } else if rate_mbps >= u32::MAX as f32 { + warn!("LTS2 bandwidth value {rate_mbps} exceeds u32::MAX; clamping to u32::MAX"); + u32::MAX } else { rate_mbps.round() as u32 // Round to nearest integer } } +fn clamp_lts2_u64_to_u32(field: &str, value: u64) -> u32 { + match u32::try_from(value) { + Ok(value) => value, + Err(_) => { + warn!("LTS2 {field} value {value} exceeds u32::MAX; clamping to u32::MAX"); + u32::MAX + } + } +} + +fn clamp_lts2_u64_to_i32(field: &str, value: u64) -> i32 { + match i32::try_from(value) { + Ok(value) => value, + Err(_) => { + warn!("LTS2 {field} value {value} exceeds i32::MAX; clamping to i32::MAX"); + i32::MAX + } + } +} + +fn lts2_rtt_millis_from_nanos(field: &str, nanos: u64) -> f32 { + let millis = nanos as f64 / 1_000_000.0; + if !millis.is_finite() || millis > f64::from(f32::MAX) { + warn!("LTS2 {field} RTT value {nanos} ns exceeds f32::MAX ms; clamping to f32::MAX"); + f32::MAX + } else { + millis as f32 + } +} + fn queue_metrics_available(config: &lqos_config::Config) -> bool { !config.queues.queue_mode.is_observe() && !queue_stats_stale() } @@ -277,22 +322,22 @@ pub(crate) fn submit_throughput_stats( tcp_retransmits_down: tcp_retransmits.down, tcp_retransmits_up: tcp_retransmits.up, cake_marks_down: if queue_metrics_available { - TOTAL_QUEUE_STATS.marks.get_down() as i32 + clamp_lts2_u64_to_i32("total cake_marks_down", TOTAL_QUEUE_STATS.marks.get_down()) } else { 0 }, cake_marks_up: if queue_metrics_available { - TOTAL_QUEUE_STATS.marks.get_up() as i32 + clamp_lts2_u64_to_i32("total cake_marks_up", TOTAL_QUEUE_STATS.marks.get_up()) } else { 0 }, cake_drops_down: if queue_metrics_available { - TOTAL_QUEUE_STATS.drops.get_down() as i32 + clamp_lts2_u64_to_i32("total cake_drops_down", TOTAL_QUEUE_STATS.drops.get_down()) } else { 0 }, cake_drops_up: if queue_metrics_available { - TOTAL_QUEUE_STATS.drops.get_up() as i32 + clamp_lts2_u64_to_i32("total cake_drops_up", TOTAL_QUEUE_STATS.drops.get_up()) } else { 0 }, @@ -418,8 +463,8 @@ pub(crate) fn submit_throughput_stats( .map(|(k, v)| crate::lts2_sys::shared_types::CircuitRetransmits { timestamp: now, circuit_hash: k, - tcp_retransmits_down: v.down as u32, - tcp_retransmits_up: v.up as u32, + tcp_retransmits_down: clamp_lts2_u64_to_u32("circuit tcp_retransmits_down", v.down), + tcp_retransmits_up: clamp_lts2_u64_to_u32("circuit tcp_retransmits_up", v.up), }) .collect::>(); if crate::lts2_sys::circuit_retransmits(&circuit_retransmits_batch).is_err() { @@ -448,7 +493,7 @@ pub(crate) fn submit_throughput_stats( Some(crate::lts2_sys::shared_types::CircuitRtt { timestamp: now, circuit_hash: *circuit_hash, - median_rtt: (median_nanos as f64 / 1_000_000.0) as f32, + median_rtt: lts2_rtt_millis_from_nanos("circuit median", median_nanos), }) }) .collect::>(); @@ -465,16 +510,28 @@ pub(crate) fn submit_throughput_stats( cake_drops.push(CircuitCakeDrops { timestamp: now, circuit_hash, - cake_drops_down: drops.get_down() as u32, - cake_drops_up: drops.get_up() as u32, + cake_drops_down: clamp_lts2_u64_to_u32( + "circuit cake_drops_down", + drops.get_down(), + ), + cake_drops_up: clamp_lts2_u64_to_u32( + "circuit cake_drops_up", + drops.get_up(), + ), }); } if marks.not_zero() { cake_marks.push(CircuitCakeMarks { timestamp: now, circuit_hash, - cake_marks_down: marks.get_down() as u32, - cake_marks_up: marks.get_up() as u32, + cake_marks_down: clamp_lts2_u64_to_u32( + "circuit cake_marks_down", + marks.get_down(), + ), + cake_marks_up: clamp_lts2_u64_to_u32( + "circuit cake_marks_up", + marks.get_up(), + ), }); } }); @@ -517,24 +574,42 @@ pub(crate) fn submit_throughput_stats( site_retransmits.push(crate::lts2_sys::shared_types::SiteRetransmits { timestamp: now, site_hash, - tcp_retransmits_down: node.current_tcp_retransmits.down as u32, - tcp_retransmits_up: node.current_tcp_retransmits.up as u32, + tcp_retransmits_down: clamp_lts2_u64_to_u32( + "site tcp_retransmits_down", + node.current_tcp_retransmits.down, + ), + tcp_retransmits_up: clamp_lts2_u64_to_u32( + "site tcp_retransmits_up", + node.current_tcp_retransmits.up, + ), }); } if queue_metrics_available && node.current_drops.not_zero() { site_cake_drops.push(crate::lts2_sys::shared_types::SiteCakeDrops { timestamp: now, site_hash, - cake_drops_down: node.current_drops.get_down() as u32, - cake_drops_up: node.current_drops.get_up() as u32, + cake_drops_down: clamp_lts2_u64_to_u32( + "site cake_drops_down", + node.current_drops.get_down(), + ), + cake_drops_up: clamp_lts2_u64_to_u32( + "site cake_drops_up", + node.current_drops.get_up(), + ), }); } if queue_metrics_available && node.current_marks.not_zero() { site_cake_marks.push(crate::lts2_sys::shared_types::SiteCakeMarks { timestamp: now, site_hash, - cake_marks_down: node.current_marks.get_down() as u32, - cake_marks_up: node.current_marks.get_up() as u32, + cake_marks_down: clamp_lts2_u64_to_u32( + "site cake_marks_down", + node.current_marks.get_down(), + ), + cake_marks_up: clamp_lts2_u64_to_u32( + "site cake_marks_up", + node.current_marks.get_up(), + ), }); } let download = node @@ -556,7 +631,7 @@ pub(crate) fn submit_throughput_stats( site_rtt.push(crate::lts2_sys::shared_types::SiteRtt { timestamp: now, site_hash, - median_rtt: (median_nanos as f64 / 1_000_000.0) as f32, + median_rtt: lts2_rtt_millis_from_nanos("site median", median_nanos), }); } }); @@ -768,6 +843,14 @@ mod tests { assert_eq!(rate_for_submission(1000.8), 1001); } + #[test] + fn test_rate_for_submission_clamps_non_finite_and_huge_rates() { + assert_eq!(rate_for_submission(f32::NAN), 1); + assert_eq!(rate_for_submission(f32::INFINITY), u32::MAX); + assert_eq!(rate_for_submission(f32::NEG_INFINITY), 1); + assert_eq!(rate_for_submission(u32::MAX as f32), u32::MAX); + } + #[test] fn test_rate_for_submission_prevents_zero() { // Should never return 0, even for very small inputs @@ -783,6 +866,35 @@ mod tests { assert_eq!(rate_for_submission(100.0), 100); } + #[test] + fn lts2_u64_to_u32_clamps_protocol_limited_fields() { + assert_eq!(clamp_lts2_u64_to_u32("test", u64::from(u32::MAX)), u32::MAX); + assert_eq!( + clamp_lts2_u64_to_u32("test", u64::from(u32::MAX) + 1), + u32::MAX + ); + } + + #[test] + fn lts2_u64_to_i32_clamps_protocol_limited_fields() { + assert_eq!( + clamp_lts2_u64_to_i32("test", u64::try_from(i32::MAX).unwrap()), + i32::MAX + ); + assert_eq!( + clamp_lts2_u64_to_i32("test", u64::try_from(i32::MAX).unwrap() + 1), + i32::MAX + ); + } + + #[test] + fn lts2_rtt_millis_keeps_large_values_finite() { + assert_eq!(lts2_rtt_millis_from_nanos("test", 1_500_000), 1.5); + let millis = lts2_rtt_millis_from_nanos("test", u64::MAX); + assert!(millis.is_finite()); + assert!(millis > 1.0e13); + } + #[test] fn queue_metrics_disabled_in_observe_mode() { let mut config = lqos_config::Config::default(); diff --git a/src/test_integration_common_ignore_subnets.py b/src/test_integration_common_ignore_subnets.py index 714df8fd..eef1c3cf 100644 --- a/src/test_integration_common_ignore_subnets.py +++ b/src/test_integration_common_ignore_subnets.py @@ -23,6 +23,8 @@ def install_common_stubs(): lqlib.exception_cpes = lambda: [] lqlib.promote_to_root_list = lambda: [] lqlib.client_bandwidth_multiplier = lambda: 1.0 + lqlib.write_compiled_topology_from_python_graph_payload = lambda *_args, **_kwargs: None + lqlib.get_libreqos_directory = lambda: "/tmp/libreqos" sys.modules["liblqos_python"] = lqlib @@ -105,8 +107,9 @@ class TestIntegrationCommonIgnoreSubnets(unittest.TestCase): self.assertEqual(len(rows), 2) self.assertEqual(rows[1][0], "client-1") - self.assertIn("203.0.113.10/32", rows[1][6]) - self.assertNotIn("100.64.1.10/32", rows[1][6]) + ipv4_column = rows[0].index("IPv4") + self.assertIn("203.0.113.10/32", rows[1][ipv4_column]) + self.assertNotIn("100.64.1.10/32", rows[1][ipv4_column]) def test_prune_does_not_require_ip_to_be_in_allowed_subnets(self): integrationCommon.allowed_subnets = lambda: ["10.0.0.0/8"] @@ -131,7 +134,8 @@ class TestIntegrationCommonIgnoreSubnets(unittest.TestCase): self.assertEqual(len(rows), 2) self.assertEqual(rows[1][0], "client-1") - self.assertIn("203.0.113.10/32", rows[1][6]) + ipv4_column = rows[0].index("IPv4") + self.assertIn("203.0.113.10/32", rows[1][ipv4_column]) if __name__ == "__main__": diff --git a/src/test_integration_visp.py b/src/test_integration_visp.py index 57a2825b..372bac20 100644 --- a/src/test_integration_visp.py +++ b/src/test_integration_visp.py @@ -1,6 +1,41 @@ import unittest +import sys +import types from unittest.mock import patch + +def install_visp_stubs(): + lqlib = types.ModuleType("liblqos_python") + lqlib.allowed_subnets = lambda: ["0.0.0.0/0", "::/0"] + lqlib.ignore_subnets = lambda: [] + lqlib.generated_pn_download_mbps = lambda: 1000 + lqlib.generated_pn_upload_mbps = lambda: 1000 + lqlib.circuit_name_use_address = lambda: False + lqlib.upstream_bandwidth_capacity_download_mbps = lambda: 1000 + lqlib.upstream_bandwidth_capacity_upload_mbps = lambda: 1000 + lqlib.find_ipv6_using_mikrotik = lambda: False + lqlib.exclude_sites = lambda: [] + lqlib.bandwidth_overhead_factor = lambda: 1.0 + lqlib.committed_bandwidth_multiplier = lambda: 1.0 + lqlib.exception_cpes = lambda: [] + lqlib.promote_to_root_list = lambda: [] + lqlib.client_bandwidth_multiplier = lambda: 1.0 + lqlib.write_compiled_topology_from_python_graph_payload = lambda *_args, **_kwargs: None + lqlib.get_libreqos_directory = lambda: "/tmp/libreqos" + lqlib.visp_client_id = lambda: "" + lqlib.visp_client_secret = lambda: "" + lqlib.visp_username = lambda: "" + lqlib.visp_password = lambda: "" + lqlib.visp_isp_id = lambda: "" + lqlib.visp_online_users_domain = lambda: "" + lqlib.visp_timeout_secs = lambda: 60 + sys.modules["liblqos_python"] = lqlib + + +install_visp_stubs() +sys.modules.pop("integrationCommon", None) +sys.modules.pop("integrationVISP", None) + from integrationVISP import ( _bulk_ipv4_candidates, _select_attachment_equipment, diff --git a/src/test_mikrotik_find_ipv6.py b/src/test_mikrotik_find_ipv6.py index 932a52e4..c1999428 100644 --- a/src/test_mikrotik_find_ipv6.py +++ b/src/test_mikrotik_find_ipv6.py @@ -7,13 +7,9 @@ import unittest sys.modules.setdefault("routeros_api", types.SimpleNamespace()) -sys.modules.setdefault( - "liblqos_python", - types.SimpleNamespace( - load_mikrotik_ipv6_routers_json=lambda: "[]", - mikrotik_ipv6_config_path=lambda: "/etc/libreqos/mikrotik_ipv6.toml", - ), -) +lqlib = sys.modules.setdefault("liblqos_python", types.SimpleNamespace()) +lqlib.load_mikrotik_ipv6_routers_json = lambda: "[]" +lqlib.mikrotik_ipv6_config_path = lambda: "/etc/libreqos/mikrotik_ipv6.toml" import mikrotikFindIPv6 from mikrotikFindIPv6 import _build_ipv4_to_ipv6_map, _mac_from_dhcpv6_duid diff --git a/src/test_xdp_mapping_recovery.py b/src/test_xdp_mapping_recovery.py index 650f4bdd..140c2861 100644 --- a/src/test_xdp_mapping_recovery.py +++ b/src/test_xdp_mapping_recovery.py @@ -1,4 +1,75 @@ import unittest +import sys +import types + + +def install_libreqos_import_stubs(): + python_check = types.ModuleType("pythonCheck") + python_check.checkPythonVersion = lambda: None + sys.modules["pythonCheck"] = python_check + + deepdiff = types.ModuleType("deepdiff") + deepdiff.DeepDiff = lambda *_args, **_kwargs: {} + sys.modules["deepdiff"] = deepdiff + + lqlib = types.ModuleType("liblqos_python") + lqlib.is_lqosd_alive = lambda: True + lqlib.clear_ip_mappings = lambda: None + lqlib.delete_ip_mapping = lambda *_args, **_kwargs: None + lqlib.validate_shaped_devices = lambda: "OK" + lqlib.is_libre_already_running = lambda: False + lqlib.create_lock_file = lambda: None + lqlib.free_lock_file = lambda: None + lqlib.add_ip_mapping = lambda *_args, **_kwargs: None + + class DummyBatchedCommands: + pass + + class DummyBakery: + pass + + lqlib.BatchedCommands = DummyBatchedCommands + lqlib.check_config = lambda: None + lqlib.sqm = lambda: "cake" + lqlib.upstream_bandwidth_capacity_download_mbps = lambda: 1000 + lqlib.upstream_bandwidth_capacity_upload_mbps = lambda: 1000 + lqlib.interface_a = lambda: "eth0" + lqlib.interface_b = lambda: "eth1" + lqlib.enable_actual_shell_commands = lambda: False + lqlib.use_bin_packing_to_balance_cpu = lambda: False + lqlib.queue_mode = lambda: "shape" + lqlib.run_shell_commands_as_sudo = lambda: False + lqlib.generated_pn_download_mbps = lambda: 1000 + lqlib.generated_pn_upload_mbps = lambda: 1000 + lqlib.queues_available_override = lambda: 0 + lqlib.on_a_stick = lambda: False + lqlib.get_tree_weights = lambda: {} + lqlib.get_weights = lambda: {} + lqlib.is_network_flat = lambda: False + lqlib.get_libreqos_directory = lambda: "/tmp/libreqos" + lqlib.enable_insight_topology = lambda: False + lqlib.is_insight_enabled = lambda: False + lqlib.scheduler_error = lambda *_args, **_kwargs: None + lqlib.xdp_ip_mapping_capacity = lambda: 1024 + lqlib.overrides_circuit_adjustments_effective = lambda: [] + lqlib.automatic_import_uisp = lambda: False + lqlib.automatic_import_splynx = lambda: False + lqlib.automatic_import_powercode = lambda: False + lqlib.automatic_import_sonar = lambda: False + lqlib.automatic_import_wispgate = lambda: False + lqlib.automatic_import_netzur = lambda: False + lqlib.automatic_import_visp = lambda: False + lqlib.topology_import_ingress_enabled = lambda: False + lqlib.plan_top_level_cpu_bins = lambda *_args, **_kwargs: {} + lqlib.plan_class_identities = lambda *_args, **_kwargs: {} + lqlib.fast_queues_fq_codel = lambda: False + lqlib.shaping_cpu_count = lambda: 16 + lqlib.Bakery = DummyBakery + sys.modules["liblqos_python"] = lqlib + + +install_libreqos_import_stubs() +sys.modules.pop("LibreQoS", None) import LibreQoS
- - + + Your Username
- Your Username - - -
+ +
Your password