From 7b453d852f0bdbe4f2094dfe33caa87b2b0265b8 Mon Sep 17 00:00:00 2001 From: UNITRONIX <36471318+UNITRONIX@users.noreply.github.com> Date: Fri, 17 Apr 2026 23:21:11 +0200 Subject: [PATCH] Security hardening and audit cleanup Multiple security and maintenance fixes across components: - betterdesk-mgmt: validate peer_id format to prevent injection in connect_to_peer (reject empty/oversized/invalid chars). - betterdesk-mgmt (tauri.conf.json): tighten CSP by removing 'unsafe-eval' from script-src. - betterdesk-agent-client: increase device ID entropy from 4 to 8 bytes (BD- prefix) to reduce collision/brute-force risk. - betterdesk-server: enforce RBAC (operator+) before upgrading CDAP video WebSocket to block unauthorized access. - betterdesk-server DBs: exclude soft_deleted peers in GetPeer queries for Postgres and SQLite. - web-nodejs: add audit log housekeeping (hourly cleanup), add indices for audit_log, and implement cleanupOldAuditLogs(days) in sqlite adapter. - web-nodejs brandingService: validate logo/favicon URLs to allow only http(s) or relative paths, preventing javascript:/data: XSS/SSRF vectors. - docs: add AUDIT_BETTERDESK_2026-04-17.md (security audit summary). These changes tighten client CSP, improve input validation, increase device identifier entropy, ensure RBAC is enforced before websocket upgrades, hide soft-deleted peers from normal queries, and add audit log maintenance and DB indexes for better performance and retention management. --- .../src-tauri/src/registration.rs | 2 +- betterdesk-mgmt/src-tauri/src/commands.rs | 7 + betterdesk-mgmt/src-tauri/tauri.conf.json | 2 +- betterdesk-server/api/cdap_handlers.go | 14 + betterdesk-server/db/postgres.go | 2 +- betterdesk-server/db/sqlite.go | 2 +- docs/AUDIT_BETTERDESK_2026-04-17.md | 580 ++++++++++++++++++ web-nodejs/server.js | 8 + web-nodejs/services/brandingService.js | 9 +- web-nodejs/services/dbAdapter.js | 14 + 10 files changed, 634 insertions(+), 6 deletions(-) create mode 100644 docs/AUDIT_BETTERDESK_2026-04-17.md diff --git a/betterdesk-agent-client/src-tauri/src/registration.rs b/betterdesk-agent-client/src-tauri/src/registration.rs index 6b961f70..1e729909 100644 --- a/betterdesk-agent-client/src-tauri/src/registration.rs +++ b/betterdesk-agent-client/src-tauri/src/registration.rs @@ -196,7 +196,7 @@ pub async fn register(config: &mut AgentConfig) -> Result { let mut hasher = Sha256::new(); hasher.update(device_uid.as_bytes()); let result = hasher.finalize(); - format!("BD-{}", hex_encode(&result[..4]).to_uppercase()) + format!("BD-{}", hex_encode(&result[..8]).to_uppercase()) }; let payload = serde_json::json!({ diff --git a/betterdesk-mgmt/src-tauri/src/commands.rs b/betterdesk-mgmt/src-tauri/src/commands.rs index 4bf914de..6d7e78bf 100644 --- a/betterdesk-mgmt/src-tauri/src/commands.rs +++ b/betterdesk-mgmt/src-tauri/src/commands.rs @@ -215,6 +215,13 @@ pub async fn connect_to_peer( peer_id: String, server_url: Option, ) -> Result { + // Security: validate peer_id format to prevent injection + if peer_id.is_empty() || peer_id.len() > 64 + || !peer_id.chars().all(|c| c.is_alphanumeric() || c == '-' || c == '_') + { + return Err("Invalid peer_id format".to_string()); + } + let settings = { let s = state.settings.lock().map_err(|e| e.to_string())?; s.clone() diff --git a/betterdesk-mgmt/src-tauri/tauri.conf.json b/betterdesk-mgmt/src-tauri/tauri.conf.json index fde2d6a4..d4006b8b 100644 --- a/betterdesk-mgmt/src-tauri/tauri.conf.json +++ b/betterdesk-mgmt/src-tauri/tauri.conf.json @@ -28,7 +28,7 @@ } ], "security": { - "csp": "default-src 'self'; script-src 'self' 'unsafe-eval'; connect-src 'self' http: https: ws: wss: ipc: https://ipc.localhost; img-src 'self' data: blob: asset: https://asset.localhost; style-src 'self' 'unsafe-inline'; font-src 'self' data:; media-src 'self' blob:; object-src 'none'; frame-ancestors 'none'" + "csp": "default-src 'self'; script-src 'self'; connect-src 'self' http: https: ws: wss: ipc: https://ipc.localhost; img-src 'self' data: blob: asset: https://asset.localhost; style-src 'self' 'unsafe-inline'; font-src 'self' data:; media-src 'self' blob:; object-src 'none'; frame-ancestors 'none'" } }, "bundle": { diff --git a/betterdesk-server/api/cdap_handlers.go b/betterdesk-server/api/cdap_handlers.go index b5bcd102..8da18849 100644 --- a/betterdesk-server/api/cdap_handlers.go +++ b/betterdesk-server/api/cdap_handlers.go @@ -677,6 +677,20 @@ func (s *Server) handleCDAPVideo(w http.ResponseWriter, r *http.Request) { username := getUsernameFromCtx(r) role := getRoleFromCtx(r) + // RBAC: operator+ can access video streams (must check BEFORE WS upgrade) + effectiveRole := role + if s.cdapGw.Delegations() != nil { + if delegated := s.cdapGw.Delegations().GetEffectiveRole(username, id, "video"); delegated != "" { + if cdap.RoleLevel(delegated) > cdap.RoleLevel(effectiveRole) { + effectiveRole = delegated + } + } + } + if cdap.RoleLevel(effectiveRole) < cdap.RoleLevel("operator") { + writeJSON(w, http.StatusForbidden, map[string]string{"error": "Video access requires operator role"}) + return + } + wsConn, err := websocket.Accept(w, r, &websocket.AcceptOptions{ Subprotocols: []string{"cdap-video"}, }) diff --git a/betterdesk-server/db/postgres.go b/betterdesk-server/db/postgres.go index c71c4dfc..0ba3ffcd 100644 --- a/betterdesk-server/db/postgres.go +++ b/betterdesk-server/db/postgres.go @@ -391,7 +391,7 @@ func scanPeer(row pgx.Row) (*Peer, error) { // GetPeer returns a peer by ID, or nil if not found. func (pg *PostgresDB) GetPeer(id string) (*Peer, error) { row := pg.pool.QueryRow(pg.ctx, - `SELECT `+peerColumns+` FROM peers WHERE id = $1`, id) + `SELECT `+peerColumns+` FROM peers WHERE id = $1 AND (soft_deleted IS NULL OR soft_deleted = false)`, id) p, err := scanPeer(row) if err == pgx.ErrNoRows { return nil, nil diff --git a/betterdesk-server/db/sqlite.go b/betterdesk-server/db/sqlite.go index 7face637..f746741d 100644 --- a/betterdesk-server/db/sqlite.go +++ b/betterdesk-server/db/sqlite.go @@ -382,7 +382,7 @@ func (s *SQLiteDB) GetPeer(id string) (*Peer, error) { disabled, banned, ban_reason, banned_at, soft_deleted, deleted_at, note, tags, heartbeat_seq, device_type, linked_peer_id, display_name - FROM peers WHERE id = ?`, id).Scan( + FROM peers WHERE id = ? AND (soft_deleted IS NULL OR soft_deleted = 0)`, id).Scan( &p.ID, &p.UUID, &p.PK, &p.IP, &p.User, &p.Hostname, &p.OS, &p.Version, &p.Status, &p.NATType, &lastOnline, &createdAt, &p.Disabled, &p.Banned, diff --git a/docs/AUDIT_BETTERDESK_2026-04-17.md b/docs/AUDIT_BETTERDESK_2026-04-17.md new file mode 100644 index 00000000..5832eb9f --- /dev/null +++ b/docs/AUDIT_BETTERDESK_2026-04-17.md @@ -0,0 +1,580 @@ +# Audyt bezpieczeństwa i stabilności BetterDesk + +**Data:** 2026-04-17 +**Zakres:** Pełny audyt projektu UNITRONIX/Rustdesk-FreeConsole (BetterDesk) +**Autor:** GitHub Copilot (audyt automatyczny z ręczną weryfikacją) +**Status projektu:** v2.4.0 (instalatory), Phase 53 (RBAC + CSRF fixes) + +--- + +## 1. Streszczenie wykonawcze + +BetterDesk jest dojrzałym, wielomodułowym ekosystemem zastępującym stos RustDesk (hbbs+hbbr) własnym serwerem Go, konsolą Node.js, klientem operatorskim MGMT (Tauri), klientem agenta końcowego (Tauri), natywnym agentem Go oraz protokołem CDAP z mostami (Modbus/SNMP/REST). W ostatnich 53 fazach projekt otrzymał gruntowne utwardzenie bezpieczeństwa (RBAC 7-role, CSRF double-submit, PBKDF2, Ed25519, NaCl TCP, TOTP 2FA, audit log, rate-limiting), jednak nadal występują luki wymagające pilnej interwencji oraz znaczące braki funkcjonalne w klientach Tauri. + +### Podsumowanie ryzyka per-moduł + +| Moduł | Krytyczne | Wysokie | Średnie | Niskie | Kompletność | +|-------|:---:|:---:|:---:|:---:|:---:| +| Go Server (`betterdesk-server/`) | **4** | 6 | 8 | 5 | ~95% | +| Node.js Console (`web-nodejs/`) | 0 | 3 | 5 | 2 | ~98% | +| MGMT Client (`betterdesk-mgmt/`) | **4** | 4 | 2 | — | ~40% | +| Agent Client (`betterdesk-agent-client/`) | **3** | 3 | 3 | — | ~20% | +| Natywny Go Agent (`betterdesk-agent/`) | **1** | 2 | 2 | — | ~50% | +| Instalatory + Docker | 0 | 1 | 3 | 1 | ~95% | +| Architektura połączeń | **3** | 3 | 3 | — | — | +| **RAZEM** | **15** | **22** | **26** | **8** | — | + +### Pięć najważniejszych priorytetów (Tier 0 — do wykonania natychmiast) + +1. **TLS wszędzie dla kanałów API** — porty 21121 (Node.js RustDesk Client API) oraz 21122 (CDAP WebSocket) transmitują zdalne operacje terminala, zrzuty ekranu i klucze API w plaintext; wymusić `https://`/`wss://` w domyślnej konfiguracji. +2. **MITM w klientach Tauri** — `danger_accept_invalid_certs(true)` w `betterdesk-mgmt` i `betterdesk-agent-client` pozwala dowolnemu atakującemu na przejęcie rejestracji/sesji operatora; usunąć bezwarunkową akceptację i wprowadzić potwierdzenie użytkownika + pinning fingerprintu. +3. **JWT secret regenerowany przy restarcie Go servera** — konfig `config/config.go:170-180` generuje nowy secret jeśli brak w env, co unieważnia wszystkie sesje operatorów przy każdym restarcie; persist secret w bezpiecznym storze. +4. **Brak auth przed upgrade WebSocket w Go serverze** — `api/cdap_handlers.go:425-430` oraz `signal/ws.go` wykonują `upgrader.Upgrade()` przed weryfikacją tokenu, co umożliwia DoS przez masowe half-open connections. +5. **Localstorage token w MGMT Client** — `bd_access_token` dostępny dla dowolnego XSS w WebView Tauri; przenieść do `tauri-plugin-stronghold` lub secure IPC state. + +--- + +## 2. Architektura i analiza połączeń + +### 2.1 Mapa portów i kanałów + +| Źródło | Cel | Protokół | Port | Auth | TLS domyślnie | Rate-limit | Audit | +|--------|-----|---------|-----:|------|:------:|:------:|:------:| +| RustDesk Client | Go Signal | UDP | 21116 | Ed25519/NaCl | — | ✅ IP | ✅ | +| RustDesk Client | Go Signal | TCP | 21116 | NaCl handshake | opcjonalne (`--tls-signal`) | ✅ | ✅ | +| RustDesk Client | Go Relay | TCP | 21117 | UUID pairing | opcjonalne (`--tls-relay`) | **❌** | ⚠️ | +| RustDesk Client | Go Signal | WS | 21118 | NaCl | opcjonalne (WSS) | ✅ | ⚠️ | +| RustDesk Client | Go Relay | WS | 21119 | UUID | opcjonalne | ❌ | ⚠️ | +| RustDesk Client | Go HTTP API | TCP | 21114 | brak (heartbeat/sysinfo) | opcjonalne | ✅ | ✅ | +| RustDesk Client | Node.js Client API | TCP | 21121 | session cookie | **❌ nigdy** | ⚠️ częściowo | ⚠️ | +| Node.js Console | Go Server | HTTP | 21114 | `X-API-Key` | opcjonalne | ✅ | ✅ | +| Browser (panel) | Node.js Console | HTTPS/HTTP | 5000 | session + CSRF + TOTP | opcjonalne | ✅ | ✅ | +| MGMT Client (Tauri) | Node.js Panel | HTTPS | 5000 | session cookie (IPC proxy) | opcjonalne | ⚠️ | ⚠️ | +| MGMT Client | Go Server | HTTP | 21114 | JWT Bearer | opcjonalne | ✅ | ✅ | +| Agent Client (Tauri) | Go Server | HTTP | 21114 | API key | opcjonalne | ✅ | ✅ | +| Natywny Go Agent | Go CDAP Gateway | WS | 21122 | API key w `auth` msg | **❌ `ws://`** | ❌ | ✅ | +| CDAP Bridge | Go CDAP Gateway | WS | 21122 | API key | **❌ `ws://`** | ❌ | ✅ | +| Admin | Go Admin Console | TCP | cfg. | hasło | brak | — | — | +| Prometheus | Go `/metrics` | HTTP | 21114 | **brak auth** | opcjonalne | — | — | + +### 2.2 Krytyczne luki architektoniczne + +1. **Plaintext transmission CDAP** (port 21122) — natywny Go Agent oraz mosty Modbus/SNMP/REST łączą się domyślnie `ws://`, przesyłając output terminala, zawartość plików, zrzuty ekranu i klucze API bez szyfrowania. Wymusić `wss://` i odrzucać `ws://` w produkcji. +2. **Brak mutual auth Node.js ↔ Go** — Node.js posiada `API_KEY`, ale Go nie weryfikuje tożsamości klienta przez cert pinning ani mTLS; każdy kto uzyska API key ma pełny dostęp. +3. **Relay brak rate-limitingu** — port 21117 nie limituje liczby jednoczesnych sesji per IP, co umożliwia resource exhaustion (OOM przez `io.Copy` bufory). +4. **Admin TCP console** (`admin/server.go`) — nie wymusza bind do `127.0.0.1`; przy złej konfiguracji nasłuchuje publicznie. +5. **`/metrics` bez auth** — eksponuje liczniki/histogramy, w tym nazwy peerów, potencjalnie umożliwiając enumerację urządzeń. +6. **SameSite=Lax na session cookie Node.js** — GET CSRF nadal możliwy na endpointach logout/verify (obecnie bezpieczne, bo read-only, ale ryzyko regresji). + +### 2.3 Rekomendacje hardeningu architektury + +- **Tier 1 (do 2 tygodni):** + - Wymusić TLS dla portów 21121 i 21122 w instalatorach (generowanie samosignowanego certu przy setupie). + - Dodać `heartbeatLimiter` w Node.js client API (21121). + - Dodać rate-limit per-IP na relay (port 21117) — max 20 równoczesnych sesji. + - `/metrics` za basic-auth lub bind do `127.0.0.1`. + - Wymusić bind admin console do `127.0.0.1` w kodzie. +- **Tier 2 (do miesiąca):** + - Cert pinning Node.js → Go (SHA256 fingerprint pierwszego połączenia). + - Dodać `StrictTransportSecurity` nagłówek w Node.js. + - Podnieść SameSite=Strict dla session cookie (testy regresji logowania). +- **Tier 3 (strategicznie):** + - mTLS dla kanału Node.js ↔ Go z automatyczną rotacją certów. + - Audit logging wszystkich komend CDAP (metadata bez payloadu). + +--- + +## 3. Audyt per-moduł + +### 3.1 Go Server (`betterdesk-server/`) + +~100k LOC, 90 plików, SQLite + PostgreSQL, Ed25519, NaCl, PBKDF2 100k, JWT HMAC-SHA256, TOTP RFC 6238, rate-limiting, RBAC 7-role. + +#### Znaleziska Krytyczne (4) + +| # | Plik:linia | Problem | Wpływ | Rekomendacja | +|---|---|---|---|---| +| **C1** | `api/cdap_handlers.go:425-430`, `signal/ws.go` | Brak auth przed `upgrader.Upgrade()` | DoS przez masowe half-open WS + enumeracja ścieżek | Autoryzuj tokenem przed upgrade; przy błędzie zwróć 401 bez upgrade | +| **C2** | `config/config.go:170-180`, `main.go` | JWT secret regenerowany przy każdym restarcie | Wszystkie tokeny operatorów unieważniane, wymuszone wylogowanie, ryzyko braku rotacji | Persist w `.jwt_secret` (chmod 600) lub w tabeli `server_config`; wygeneruj tylko jeśli brak | +| **C3** | `signal/handler.go:1070-1080` | Race condition w `pendingRelayUUIDs` przy równoczesnych `RequestRelay` + timeout | Możliwy mismatch UUID → nieudane parowanie relay lub crosstalk | Użyj mutex per-UUID + atomic store-if-absent | +| **C4** | `admin/server.go` | TCP admin console nie wymusza bind do loopback | RCE przez słabe hasło administracyjne z sieci | W kodzie `net.Listen("tcp", "127.0.0.1:"+port)`; opcjonalnie flaga `--admin-bind` | + +#### Znaleziska Wysokie (6) + +| # | Plik:linia | Problem | Rekomendacja | +|---|---|---|---| +| H1 | `api/auth_handlers.go:1491` | Bcrypt error przekazywany do klienta (info leak) | Loguj wewnętrznie, zwracaj `"invalid credentials"` | +| H2 | `api/server.go` middleware | `X-Forwarded-For` zaufany zawsze | Dodać gate `TRUST_PROXY=true/false`, parsować tylko wtedy | +| H3 | `api/server.go` | Query param `?api_key=...` nadal akceptowany | Logowanie w access logach; deprecate + log warning | +| H4 | `db/sqlite.go`, `db/postgres.go` | `soft_deleted` peerzy widoczni w query `GetPeer` | Dodać `WHERE soft_deleted=0` do wszystkich SELECTów oprócz admin audytu | +| H5 | `auth/totp.go` | Kody odzyskania TOTP niezaimplementowane (stub) | Generować 10 jednorazowych kodów bcrypt-hashed przy włączaniu 2FA | +| H6 | `config/config.go` | Brak walidacji wartości env (np. `LOG_LEVEL=DROP_TABLE`) | Enum whitelist dla każdej opcji enum | + +#### Znaleziska Średnie (8) + +| # | Obszar | Opis | +|---|---|---| +| M1 | `auth/password.go` | PBKDF2 100k OK, ale argon2id byłby nowocześniejszy | +| M2 | `ratelimit/*` | Brak normalizacji IPv6 (/64) — każdy sufiks traktowany osobno | +| M3 | `db/postgres.go` | Escapowanie `%`/`_` obecne, ale brak w kilku zapytaniach autorskich | +| M4 | `signal/handler.go` | `tcpPunchConns sync.Map` ma TTL 2min, ale brak limit liczby wpisów przy burst | +| M5 | `auth/jwt.go` | Brak blacklisty JTI po `logout` — token ważny do expiry | +| M6 | `relay/server.go` | `SetDeadline` zamiast `context.Context` → trudniej shutdown | +| M7 | `cdap/audio.go` | Brak walidacji rozmiaru ramki audio (potencjalny OOM) | +| M8 | Instalator | `.admin_credentials` plaintext jeśli `STORE_ADMIN_CREDENTIALS=true` | + +#### Znaleziska Niskie (5) + +- L1: `/metrics` bez auth (expose device IDs w labelach) +- L2: Log injection przez peer ID w `log.Printf("peer %s ...", id)` bez sanitizacji CR/LF +- L3: `GET /api/config/enum/:key` brak rate limit +- L4: `GetRelayServers()` już waliduje host ≥ 2 znaków, ale brak walidacji portu (range 1-65535) +- L5: Audit log nie zawiera `session_id` dla korelacji zdarzeń z sesją operatora + +#### Ocena sumaryczna + +Fundamenty bezpieczeństwa są solidne (RBAC, rate-limit, PBKDF2, constant-time compare, NaCl, Ed25519). Główne deficyty to niepersystowany JWT secret, niechroniony upgrade WebSocket i brak TLS-everywhere w defaults. **Ocena: 7/10.** + +--- + +### 3.2 Node.js Console (`web-nodejs/`) + +Express + EJS + better-sqlite3 / pg + csrf-csrf + helmet + express-rate-limit + bcrypt + otplib. + +#### Znaleziska Wysokie (3) + +| # | Plik:linia | Problem | Rekomendacja | +|---|---|---|---| +| H1 | `routes/settings.routes.js:144`, `public/js/settings.js:646` | `logoUrl` bez walidacji schematu (`javascript:`, `data:`, `file://` → XSS/SSRF) | Whitelist `http://`, `https://`, `/uploads/` przez `URL` constructor | +| H2 | `routes/settings.routes.js:142-148` | Branding akceptuje ~30 pól bez whitelisty; `logoSvg` sanitizowany regexem (omija `