diff --git a/SECURITY.md b/SECURITY.md new file mode 100644 index 00000000..64151bf5 --- /dev/null +++ b/SECURITY.md @@ -0,0 +1,99 @@ +# Security Policy + +> This document describes how to report security vulnerabilities in BetterDesk +> and what to expect from the maintainers in return. + +## Supported Versions + +Only the latest minor release on the `main` branch receives active security +patches. Older tagged releases may receive backports for critical issues at the +maintainers' discretion. + +| Version | Supported | +| ------------- | ------------------ | +| `main` | :white_check_mark: | +| latest tag | :white_check_mark: | +| older tags | :x: (best-effort) | + +## Reporting a Vulnerability + +**Please do NOT open a public GitHub issue for security problems.** + +Preferred channels (in order): + +1. **GitHub Security Advisory (private):** + +2. **Encrypted email:** `security@betterdesk.invalid` *(replace with your real + address before publishing; PGP key TBD)* + +Please include, where possible: + +- A clear description of the issue and its impact. +- A reproduction recipe (PoC, request/response, or test case). +- Affected versions / commits / deployment shape (Docker single-container, + multi-container, bare-metal install, etc.). +- Any logs, screenshots or traffic captures \u2014 with sensitive data redacted. + +## Response Targets + +- **Acknowledgement:** within **7 days** of report. +- **Triage decision:** within **14 days** (severity, scope, fix plan). +- **Coordinated disclosure window:** **90 days** maximum from acknowledgement, + shortened if the issue is being actively exploited. +- **Credit:** at your option, in the release notes / advisory. + +If you do not get an acknowledgement within 7 days, please ping the repository +maintainers via a *public* but vague issue ("waiting on security report +acknowledgement"). Do not disclose any details. + +## Scope + +In scope: + +- The Go server (`betterdesk-server/`) including signal, relay, HTTP/WS API, + CDAP gateway, BD-MGMT WebSocket, and the database adapters. +- The Node.js web console (`web-nodejs/`) including the RustDesk-compatible + client API on port `21121`, the panel routes, and the WS push services. +- The Tauri MGMT and Agent clients (`betterdesk-mgmt/`, `betterdesk-agent-client/`). +- The native Go agent (`betterdesk-agent/`). +- The CDAP SDKs and reference bridges (`sdks/`, `bridges/`). +- The ALL-IN-ONE install scripts (`betterdesk.sh`, `betterdesk.ps1`, + `betterdesk-docker.sh`) and the Docker compose files. + +Out of scope: + +- Vulnerabilities that require an attacker who already controls the host + operating system, the database file, or the operator's browser session. +- Findings that only apply to legacy Rust binaries in `archive/` or to forks. +- Denial-of-service from raw network flooding without an amplification vector. +- Self-XSS, missing security headers on documentation pages, or theoretical + issues without a concrete exploitation path. +- Reports generated solely by automated scanners without manual validation. + +## Hardening Defaults + +BetterDesk ships with the following defaults that reduce the blast radius of +typical issues. Operators should keep them enabled unless they have a specific +reason to relax them: + +- TOTP-based 2FA enforced on the web panel for all roles. +- WebSocket origin allowlist for the API events endpoint (`API_WS_ALLOWED_ORIGINS`). +- `trust proxy` disabled by default; must be opted in explicitly when behind a + reverse proxy. +- Local-only bind for the panel HTTP port (`HOST=127.0.0.1`); only the + RustDesk client API (`21121`) is intended for WAN exposure. +- Docker containers run with `no-new-privileges:true` and `cap_drop: [ALL]`. +- Admin password and PostgreSQL password generated with `openssl rand -hex 16`. +- TOTP bypass on the RustDesk client API requires both + `RUSTDESK_API_DISABLE_TOTP=true` **and** + `RUSTDESK_API_DISABLE_TOTP_ACKNOWLEDGED=true`, with a startup banner warning. + +## Audits + +Recent internal audits are published under `docs/security/` (e.g. +`AUDIT_PRODUCTION_2026-04-10.md`). Fixes are referenced in commit messages and +in `.github/copilot-instructions.md` under the relevant Phase. + +--- + +*This policy was last updated on 2026-04-10.* diff --git a/betterdesk-docker.sh b/betterdesk-docker.sh index 04dfd08b..6833384c 100644 --- a/betterdesk-docker.sh +++ b/betterdesk-docker.sh @@ -599,7 +599,8 @@ EOF admin_password=$(cut -d: -f2 "$DATA_DIR/.admin_credentials" 2>/dev/null) fi if [ -z "$admin_password" ]; then - admin_password=$(openssl rand -base64 12 | tr -d '/+=' | head -c 16) + # SECURITY (audit fix M-05, 2026-04-10): full hex entropy + admin_password=$(openssl rand -hex 16) # Only clean auth.db on FRESH install (no existing credentials) if docker volume inspect "${PROJECT_NAME:-betterdesk}_console_data" >/dev/null 2>&1; then print_info "Cleaning old auth database from console_data volume..." @@ -817,7 +818,8 @@ create_admin_user() { # Use the password generated during compose file creation local admin_password="${DOCKER_ADMIN_PASSWORD}" if [ -z "$admin_password" ]; then - admin_password=$(openssl rand -base64 12 | tr -d '/+=' | head -c 16) + # M-05: full hex entropy + admin_password=$(openssl rand -hex 16) fi # Wait for database to be created @@ -1280,7 +1282,8 @@ do_reset_password() { case $pw_choice in 1) - new_password=$(openssl rand -base64 12 | tr -d '/+=' | head -c 16) + # M-05: full hex entropy + new_password=$(openssl rand -hex 16) ;; 2) echo "" diff --git a/betterdesk-server/api/auth_handlers.go b/betterdesk-server/api/auth_handlers.go index b377b43d..37ecbb18 100644 --- a/betterdesk-server/api/auth_handlers.go +++ b/betterdesk-server/api/auth_handlers.go @@ -194,6 +194,16 @@ func (s *Server) handleLogin(w http.ResponseWriter, r *http.Request) { return } + // SECURITY (audit fix H-03, 2026-04-10): per-username rate limiting in + // addition to per-IP. Defeats credential-stuffing from a rotating IP pool + // hammering a single high-value account (e.g. "admin"). + if s.loginLimiter != nil && !s.loginLimiter.Allow("user:"+strings.ToLower(body.Username)) { + writeJSON(w, http.StatusTooManyRequests, map[string]string{ + "error": "Too many login attempts. Please try again later.", + }) + return + } + user, err := s.db.GetUser(body.Username) if err != nil { writeJSON(w, http.StatusInternalServerError, map[string]string{"error": "Internal error"}) @@ -268,6 +278,16 @@ func (s *Server) handleLogin2FA(w http.ResponseWriter, r *http.Request) { writeJSON(w, http.StatusUnauthorized, map[string]string{"error": "Invalid or expired partial token"}) return } + + // H-03: per-username 2FA rate limiting (in addition to per-IP). + if claims != nil && claims.Username != "" { + if s.loginLimiter != nil && !s.loginLimiter.Allow("user:"+strings.ToLower(claims.Username)) { + writeJSON(w, http.StatusTooManyRequests, map[string]string{ + "error": "Too many 2FA attempts. Please try again later.", + }) + return + } + } if claims.Role != "__2fa_pending__" { writeJSON(w, http.StatusBadRequest, map[string]string{"error": "Not a 2FA partial token"}) return @@ -844,19 +864,45 @@ func (s *Server) extractOrgIDFromRequest(r *http.Request) string { return claims.OrgID } +// redactPathSegment masks high-cardinality identifiers in URL paths so that +// device IDs / user IDs do not leak verbatim into access logs (audit fix L-04). +// Currently rewrites: +// - /api/peers/{id}/... -> /api/peers//... +// - /api/cdap/devices/{id}/ -> /api/cdap/devices// +// - /ws/bd-mgmt/{id} -> /ws/bd-mgmt/ +func redactPathSegment(p string) string { + for _, prefix := range []string{"/api/peers/", "/api/cdap/devices/", "/ws/bd-mgmt/", "/api/tokens/", "/api/users/", "/api/orgs/"} { + if !strings.HasPrefix(p, prefix) { + continue + } + rest := p[len(prefix):] + slash := strings.IndexByte(rest, '/') + if slash < 0 { + return prefix + "" + } + return prefix + "" + rest[slash:] + } + return p +} + // authMiddleware replaces the old apiKeyMiddleware. // It authenticates every request and attaches role + username to the context. // Public endpoints are excluded from authentication. func (s *Server) authMiddleware(next http.Handler) http.Handler { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - // Log ALL incoming HTTP requests for debugging - log.Printf("[api] %s %s from %s", r.Method, r.URL.Path, s.remoteIP(r)) + // Log incoming HTTP requests. SECURITY (audit fix L-04, 2026-04-10): + // - skip noisy public probes (heartbeat / sysinfo / metrics / health) + // - redact /peers/{id} segments so device IDs do not leak into logs + path := r.URL.Path + if path != "/api/heartbeat" && path != "/api/sysinfo" && path != "/api/sysinfo_ver" && + path != "/metrics" && path != "/api/health" { + log.Printf("[api] %s %s from %s", r.Method, redactPathSegment(path), s.remoteIP(r)) + } // Limit request body size to 1 MB for all requests (S10) r.Body = http.MaxBytesReader(w, r.Body, 1<<20) // Public endpoints — no auth required - path := r.URL.Path if path == "/api/health" || path == "/metrics" || path == "/api/auth/login" || path == "/api/auth/login/2fa" || path == "/api/server/pubkey" || path == "/api/server/stats" || diff --git a/betterdesk-server/api/bd_mgmt_handlers.go b/betterdesk-server/api/bd_mgmt_handlers.go index a8edf93f..a7ec3e38 100644 --- a/betterdesk-server/api/bd_mgmt_handlers.go +++ b/betterdesk-server/api/bd_mgmt_handlers.go @@ -294,6 +294,11 @@ func (s *Server) handleBdMgmt(w http.ResponseWriter, r *http.Request) { log.Printf("[bd-mgmt] WebSocket accept error for %s: %v", deviceID, err) return } + // SECURITY (audit fix I-02, 2026-04-10): cap inbound WebSocket frames at + // 16 MiB so a misbehaving / hostile device cannot exhaust server memory + // by streaming an unbounded JSON payload. The CDAP gateway has its own + // 8 MiB cap; this channel allows slightly larger management blobs. + conn.SetReadLimit(16 << 20) ctx, cancel := context.WithCancel(r.Context()) session := &bdMgmtSession{ diff --git a/betterdesk-server/api/server.go b/betterdesk-server/api/server.go index 15368b2f..8a7d5e76 100644 --- a/betterdesk-server/api/server.go +++ b/betterdesk-server/api/server.go @@ -56,6 +56,10 @@ type Server struct { jwtManager *auth.JWTManager loginLimiter *ratelimit.IPLimiter heartbeatLimiter *ratelimit.IPLimiter // BD-2026-001: rate-limit heartbeat/sysinfo + // SECURITY (audit fix M-07, 2026-04-10): rate-limit public enrollment and + // branding endpoints to deter device-ID enumeration and config probing. + enrollmentLimiter *ratelimit.IPLimiter + brandingLimiter *ratelimit.IPLimiter keyPair *crypto.KeyPair // Ed25519 keypair for signing cdapGw *cdap.Gateway // CDAP gateway (nil if CDAP disabled) clientTFASessions *tfaSessionStore @@ -74,10 +78,73 @@ func New(cfg *config.Config, database db.Database, peerMap *peer.Map, relaySrv * version: version, loginLimiter: ratelimit.NewIPLimiter(5, 5*time.Minute, 10*time.Minute), heartbeatLimiter: ratelimit.NewIPLimiter(20, 60*time.Second, 5*time.Minute), // BD-2026-001: 20 req/min per IP + enrollmentLimiter: ratelimit.NewIPLimiter(20, 1*time.Minute, 5*time.Minute), // M-07: 20/min per IP, 5-min block + brandingLimiter: ratelimit.NewIPLimiter(60, 1*time.Minute, 5*time.Minute), // M-07: 60/min per IP clientTFASessions: newTFASessionStore(), } } +// rateLimitPublic wraps a public (no-auth) handler with the supplied IP limiter. +// Returns HTTP 429 with a JSON body when the per-IP budget is exhausted. +// Used by audit fix M-07 (enrollment, branding endpoints). +func (s *Server) rateLimitPublic(lim *ratelimit.IPLimiter, h http.HandlerFunc) http.HandlerFunc { + return func(w http.ResponseWriter, r *http.Request) { + if lim != nil { + ip, _, _ := net.SplitHostPort(r.RemoteAddr) + if ip == "" { + ip = r.RemoteAddr + } + if !lim.Allow(ip) { + writeJSON(w, http.StatusTooManyRequests, map[string]string{"error": "rate limit exceeded"}) + return + } + } + h(w, r) + } +} + +// metricsGuard enforces the audit fix H-03 access policy for /metrics: +// - cfg.MetricsPublic=true => unrestricted (legacy/dev) +// - cfg.MetricsAllowlist non-empty => caller IP must match one entry +// - otherwise => caller must present a valid bearer/api-key +func (s *Server) metricsGuard(h http.HandlerFunc) http.HandlerFunc { + allowlist := s.cfg.GetMetricsAllowlist() + public := s.cfg.MetricsPublic + return func(w http.ResponseWriter, r *http.Request) { + if public { + h(w, r) + return + } + ip, _, _ := net.SplitHostPort(r.RemoteAddr) + if ip == "" { + ip = r.RemoteAddr + } + if len(allowlist) > 0 && ipInAllowlist(ip, allowlist) { + h(w, r) + return + } + // Fall back to standard auth middleware (JWT / API key). + s.authMiddleware(h).ServeHTTP(w, r) + } +} + +// ipInAllowlist reports whether ip matches any literal IP or CIDR in list. +func ipInAllowlist(ip string, list []string) bool { + parsed := net.ParseIP(ip) + if parsed == nil { + return false + } + for _, entry := range list { + if entry == ip { + return true + } + if _, cidr, err := net.ParseCIDR(entry); err == nil && cidr.Contains(parsed) { + return true + } + } + return false +} + // SetBlocklist sets the blocklist instance for the API server. func (s *Server) SetBlocklist(bl *security.Blocklist) { s.blocklist = bl @@ -277,9 +344,9 @@ func (s *Server) Start(ctx context.Context) error { mux.HandleFunc("GET /api/enrollment/mode", s.requireRole(auth.RoleAdmin, s.handleGetEnrollmentMode)) mux.HandleFunc("PUT /api/enrollment/mode", s.requireRole(auth.RoleAdmin, s.handleSetEnrollmentMode)) - // Enrollment — device self-registration (public, no auth) - mux.HandleFunc("POST /api/devices/register", s.handleDeviceRegister) - mux.HandleFunc("GET /api/devices/register/status", s.handleDeviceRegisterStatus) + // Enrollment — device self-registration (public, no auth, rate-limited via M-07) + mux.HandleFunc("POST /api/devices/register", s.rateLimitPublic(s.enrollmentLimiter, s.handleDeviceRegister)) + mux.HandleFunc("GET /api/devices/register/status", s.rateLimitPublic(s.enrollmentLimiter, s.handleDeviceRegisterStatus)) // Enrollment — operator approval (admin/operator) mux.HandleFunc("GET /api/enrollment/pending", s.requireRole(auth.RoleOperator, s.handleListPendingDevices)) @@ -287,7 +354,7 @@ func (s *Server) Start(ctx context.Context) error { mux.HandleFunc("POST /api/enrollment/reject/{id}", s.requireRole(auth.RoleOperator, s.handleRejectDevice)) // Branding (GET is public for desktop clients, POST is admin) - mux.HandleFunc("GET /api/branding", s.handleGetBranding) + mux.HandleFunc("GET /api/branding", s.rateLimitPublic(s.brandingLimiter, s.handleGetBranding)) mux.HandleFunc("POST /api/branding", s.requireRole(auth.RoleAdmin, s.handleSaveBranding)) // CDAP device management (requires CDAP gateway to be enabled) @@ -326,8 +393,10 @@ func (s *Server) Start(ctx context.Context) error { mux.HandleFunc("POST /api/bd/mgmt/{device_id}/send", s.requireRole(auth.RoleOperator, s.handleBdMgmtSend)) mux.HandleFunc("GET /api/bd/mgmt/connected", s.handleBdMgmtConnected) - // Prometheus metrics (public, no API key required) - mux.HandleFunc("GET /metrics", s.handleMetrics) + // Prometheus metrics. Gated via H-03 — see handleMetrics for the actual + // IP-allowlist / auth check. We register a wrapper here that enforces the + // policy before any metric data is exposed. + mux.HandleFunc("GET /metrics", s.metricsGuard(s.handleMetrics)) // Catch-all: return JSON 404 for unmatched routes. // Go's default ServeMux returns HTML which breaks RustDesk (Dart) client parsing. diff --git a/betterdesk-server/config/config.go b/betterdesk-server/config/config.go index f80c990b..e0dab3ea 100644 --- a/betterdesk-server/config/config.go +++ b/betterdesk-server/config/config.go @@ -85,6 +85,15 @@ type Config struct { AllowedWSOrigins string // Comma-separated allowed WebSocket origins (empty = allow all) APIAllowedWSOrigins string // Comma-separated allowed WebSocket origins for HTTP API events endpoint + // Metrics endpoint access control (audit fix H-03, 2026-04-10) + // MetricsAllowlist: comma-separated list of IP / CIDR allowed to call /metrics. + // Empty + MetricsPublic=false => /metrics requires authentication. + // Non-empty => /metrics is open to listed IPs only, no auth. + // MetricsPublic: when true, /metrics is reachable without auth from anywhere + // (legacy behavior). Off by default. + MetricsAllowlist string + MetricsPublic bool + // TLS for signal/relay/api (Phase 3 + Phase 21) TLSSignal bool // Enable TLS on TCP signal (:21116) and WS signal (:21118) TLSRelay bool // Enable TLS on TCP relay (:21117) and WS relay (:21119) @@ -248,6 +257,12 @@ func (c *Config) LoadEnv() { if v := os.Getenv("API_WS_ALLOWED_ORIGINS"); v != "" { c.APIAllowedWSOrigins = v } + if v := os.Getenv("METRICS_IP_ALLOWLIST"); v != "" { + c.MetricsAllowlist = v + } + if strings.ToUpper(os.Getenv("METRICS_PUBLIC")) == "Y" || strings.ToUpper(os.Getenv("METRICS_PUBLIC")) == "YES" || os.Getenv("METRICS_PUBLIC") == "1" || strings.ToUpper(os.Getenv("METRICS_PUBLIC")) == "TRUE" { + c.MetricsPublic = true + } if strings.ToUpper(os.Getenv("TLS_SIGNAL")) == "Y" { c.TLSSignal = true } @@ -370,6 +385,23 @@ func (c *Config) GetAPIAllowedWSOrigins() []string { return result } +// GetMetricsAllowlist returns the parsed list of IP / CIDR allowed to call /metrics. +// Used together with MetricsPublic to decide whether to require authentication. +func (c *Config) GetMetricsAllowlist() []string { + if c.MetricsAllowlist == "" { + return nil + } + parts := strings.Split(c.MetricsAllowlist, ",") + result := make([]string, 0, len(parts)) + for _, o := range parts { + o = strings.TrimSpace(o) + if o != "" { + result = append(result, o) + } + } + return result +} + // HasTLSCert returns true if both TLS certificate and key files are configured. func (c *Config) HasTLSCert() bool { return c.TLSCertFile != "" && c.TLSKeyFile != "" diff --git a/betterdesk.sh b/betterdesk.sh index 3a3d6189..324d606a 100644 --- a/betterdesk.sh +++ b/betterdesk.sh @@ -1194,7 +1194,10 @@ setup_postgresql_database() { # Generate password if not set if [ -z "$POSTGRESQL_PASS" ]; then - POSTGRESQL_PASS=$(openssl rand -base64 16 | tr -d '/+=' | head -c 16) + # SECURITY (audit fix M-05, 2026-04-10): use hex (4 bits/char, no + # alphabet shrinking) instead of base64+tr+truncate which lost a few + # entropy bits per character. + POSTGRESQL_PASS=$(openssl rand -hex 16) print_info "Generated PostgreSQL password" fi @@ -1511,8 +1514,8 @@ install_nodejs_console() { rm -f "$CONSOLE_PATH/data/auth.db" "$CONSOLE_PATH/data/auth.db-wal" "$CONSOLE_PATH/data/auth.db-shm" fi - # Generate admin password for Node.js console - ADMIN_PASSWORD=$(openssl rand -base64 12 | tr -d '/+=' | head -c 16) + # Generate admin password for Node.js console (M-05: full hex entropy) + ADMIN_PASSWORD=$(openssl rand -hex 16) local nodejs_admin_password="$ADMIN_PASSWORD" # Create sentinel file so ensureDefaultAdmin() force-updates the password @@ -3105,7 +3108,8 @@ do_reset_password() { case $pw_choice in 1) - new_password=$(openssl rand -base64 12 | tr -d '/+=' | head -c 16) + # M-05: full hex entropy + new_password=$(openssl rand -hex 16) ;; 2) echo "" @@ -4559,14 +4563,17 @@ do_migrate_database() { fi print_step "Running Node.js → Go migration..." - local cmd="$migrate_bin -mode nodejs2go -src $src_db" + # SECURITY (audit fix M-04, 2026-04-10): use a bash array + direct exec + # instead of cmd-string + eval to avoid shell injection if any input + # contains spaces / metacharacters. + local args=("-mode" "nodejs2go" "-src" "$src_db") if [ -f "$auth_db" ]; then - cmd="$cmd -node-auth $auth_db" + args+=("-node-auth" "$auth_db") fi if [ -n "$dst_db" ]; then - cmd="$cmd -dst $dst_db" + args+=("-dst" "$dst_db") fi - eval "$cmd" 2>&1 + "$migrate_bin" "${args[@]}" 2>&1 if [ $? -eq 0 ]; then print_success "Node.js → Go migration completed successfully!" diff --git a/docker-compose.quick.yml b/docker-compose.quick.yml index 1e0587ba..324c3e0e 100644 --- a/docker-compose.quick.yml +++ b/docker-compose.quick.yml @@ -44,6 +44,13 @@ services: networks: - betterdesk-net restart: unless-stopped + # SECURITY (audit fix L-02, 2026-04-10): no privilege escalation, drop + # all Linux capabilities — Go server only needs to bind ports + write + # to the mounted volume. + security_opt: + - no-new-privileges:true + cap_drop: + - ALL healthcheck: test: ["CMD", "wget", "-q", "--spider", "http://localhost:21114/api/health"] interval: 30s @@ -85,6 +92,11 @@ services: server: condition: service_healthy restart: unless-stopped + # L-02: drop caps + no privilege escalation for Node.js console + security_opt: + - no-new-privileges:true + cap_drop: + - ALL healthcheck: test: ["CMD", "wget", "-q", "--spider", "http://localhost:5000/login"] interval: 30s diff --git a/docker-compose.single.yml b/docker-compose.single.yml index a723750d..bded0d43 100644 --- a/docker-compose.single.yml +++ b/docker-compose.single.yml @@ -74,6 +74,13 @@ services: # - ADMIN_USERNAME=admin # - ADMIN_PASSWORD=YourSecurePassword123 restart: unless-stopped + # SECURITY (audit fix L-02, 2026-04-10): minimal capabilities, no + # privilege escalation. All-in-one container still only needs to bind + # ports inside its own netns and write to mounted volumes. + security_opt: + - no-new-privileges:true + cap_drop: + - ALL healthcheck: test: ["CMD-SHELL", "curl -sf http://localhost:21114/api/health && curl -sf http://localhost:5000/health"] interval: 30s @@ -93,6 +100,11 @@ services: volumes: - postgres-data:/var/lib/postgresql/data restart: unless-stopped + # L-02: drop caps for PostgreSQL too + security_opt: + - no-new-privileges:true + cap_drop: + - ALL healthcheck: test: ["CMD-SHELL", "pg_isready -U betterdesk"] interval: 10s diff --git a/docker-compose.yml b/docker-compose.yml index 68c99371..bf002458 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -42,6 +42,13 @@ services: networks: - betterdesk-net restart: unless-stopped + # SECURITY (audit fix L-02, 2026-04-10): minimal capabilities, no privilege + # escalation. The Go server only needs to bind low ports inside the + # container namespace and write to the shared volume — no extra caps. + security_opt: + - no-new-privileges:true + cap_drop: + - ALL healthcheck: test: ["CMD", "curl", "-sf", "http://localhost:21114/api/health"] interval: 30s @@ -89,6 +96,13 @@ services: networks: - betterdesk-net restart: unless-stopped + # SECURITY (audit fix L-02, 2026-04-10): drop all caps for the Node.js + # console. Node only needs to listen on TCP/5000+21121 inside its own + # netns and write to /app/data — no host-level privileges. + security_opt: + - no-new-privileges:true + cap_drop: + - ALL depends_on: server: condition: service_healthy diff --git a/web-nodejs/config/config.js b/web-nodejs/config/config.js index 614b12a9..0bda9ace 100644 --- a/web-nodejs/config/config.js +++ b/web-nodejs/config/config.js @@ -143,6 +143,14 @@ module.exports = { // - rely on device bans / API key for admin endpoints. rustdeskApiDisableTotp: (process.env.RUSTDESK_API_DISABLE_TOTP || 'false').toLowerCase() === 'true', + // H-04 mitigation (audit 2026-04-10): + // Setting RUSTDESK_API_DISABLE_TOTP=true alone is no longer sufficient — + // the operator must also opt in explicitly with + // RUSTDESK_API_DISABLE_TOTP_ACKNOWLEDGED=true, confirming they have read + // and accepted the WAN-port 2FA bypass risk. Without the ACK flag the + // bypass is ignored and TOTP is enforced normally on :21121. + rustdeskApiDisableTotpAck: (process.env.RUSTDESK_API_DISABLE_TOTP_ACKNOWLEDGED || 'false').toLowerCase() === 'true', + // HTTPS / SSL httpsEnabled: (process.env.HTTPS_ENABLED || 'false').toLowerCase() === 'true', httpsPort: parseInt(process.env.HTTPS_PORT, 10) || 5443, diff --git a/web-nodejs/middleware/rateLimiter.js b/web-nodejs/middleware/rateLimiter.js index 1754a43c..b5b67a1e 100644 --- a/web-nodejs/middleware/rateLimiter.js +++ b/web-nodejs/middleware/rateLimiter.js @@ -5,8 +5,15 @@ const rateLimit = require('express-rate-limit'); const config = require('../config/config'); +const defaultKeyGenerator = (req) => req.ip || req.headers['x-forwarded-for'] || 'unknown'; + /** - * General API rate limiter + * General API rate limiter. + * + * SECURITY (audit fix M-03, 2026-04-10): the previous Referer-based skip was + * removed because Referer is fully client-controlled. High-frequency widget / + * dashboard refresh endpoints now have their own higher-quota limiter + * (`widgetLimiter`) that the panel routes opt into explicitly. */ const apiLimiter = rateLimit({ windowMs: config.rateLimitWindowMs, @@ -17,18 +24,24 @@ const apiLimiter = rateLimit({ success: false, error: 'Too many requests. Please try again later.' }, - keyGenerator: (req) => { - return req.ip || req.headers['x-forwarded-for'] || 'unknown'; + keyGenerator: defaultKeyGenerator +}); + +/** + * Widget / dashboard refresh limiter. Higher quota (600 req/min by default) + * because the panel polls many widgets in parallel. Still authenticated — + * mount only on routes that require an active session. + */ +const widgetLimiter = rateLimit({ + windowMs: 60 * 1000, + max: parseInt(process.env.WIDGET_RATE_LIMIT_MAX, 10) || 600, + standardHeaders: true, + legacyHeaders: false, + message: { + success: false, + error: 'Too many widget requests. Please slow down.' }, - skip: (req) => { - // Skip rate limiting for widget data endpoints from same-origin panel requests - // These are internal dashboard/widget refresh calls, not external abuse - const widgetPaths = ['/api/stats', '/api/server/status', '/api/devices', '/api/audit/conn']; - if (widgetPaths.some(p => req.path.startsWith(p)) && req.headers.referer && req.session && req.session.userId) { - return true; - } - return false; - } + keyGenerator: defaultKeyGenerator }); /** @@ -64,6 +77,7 @@ const passwordChangeLimiter = rateLimit({ module.exports = { apiLimiter, + widgetLimiter, loginLimiter, passwordChangeLimiter }; diff --git a/web-nodejs/routes/rustdesk-api.routes.js b/web-nodejs/routes/rustdesk-api.routes.js index b588fece..f3b38acd 100644 --- a/web-nodejs/routes/rustdesk-api.routes.js +++ b/web-nodejs/routes/rustdesk-api.routes.js @@ -1309,7 +1309,12 @@ router.post('/api/login', async (req, res) => { // opted in via RUSTDESK_API_DISABLE_TOTP=true, skip 2FA on this // (RustDesk-only) endpoint and issue an access token directly. // The web panel routes still enforce TOTP independently. - if (config.rustdeskApiDisableTotp) { + // + // SECURITY (audit fix H-04, 2026-04-10): the bypass also requires + // RUSTDESK_API_DISABLE_TOTP_ACKNOWLEDGED=true to confirm the + // operator understands the WAN-facing risk on :21121. Without the + // ACK flag TOTP is enforced normally even if DISABLE_TOTP is set. + if (config.rustdeskApiDisableTotp && config.rustdeskApiDisableTotpAck) { authService.recordAttempt(username, ip, true); const token = await authService.generateAccessToken(user.id, clientId, clientUuid, ip); await db.updateLastLogin(user.id); diff --git a/web-nodejs/routes/system.routes.js b/web-nodejs/routes/system.routes.js index 24d36a2b..adea273e 100644 --- a/web-nodejs/routes/system.routes.js +++ b/web-nodejs/routes/system.routes.js @@ -29,7 +29,7 @@ function safeExec(cmd, timeout) { // ─── GET /api/system/info ───────────────────────────────────────────────────── -router.get('/api/system/info', requireAuth, (req, res) => { +router.get('/api/system/info', requireAuth, requirePermission('metrics.view'), (req, res) => { try { const result = { processes: [], disks: [] }; @@ -109,7 +109,7 @@ router.get('/api/system/info', requireAuth, (req, res) => { // ─── GET /api/logs/recent ───────────────────────────────────────────────────── -router.get('/api/logs/recent', requireAuth, (req, res) => { +router.get('/api/logs/recent', requireAuth, requirePermission('metrics.view'), (req, res) => { try { const source = req.query.source || 'console'; const limit = Math.min(parseInt(req.query.limit, 10) || 50, 200); @@ -159,7 +159,7 @@ router.get('/api/logs/recent', requireAuth, (req, res) => { // ─── GET /api/database/stats ────────────────────────────────────────────────── -router.get('/api/database/stats', requireAuth, async (req, res) => { +router.get('/api/database/stats', requireAuth, requirePermission('metrics.view'), async (req, res) => { try { const db = require('../services/dbAdapter'); const config = require('../config/config'); @@ -223,7 +223,7 @@ router.get('/api/database/stats', requireAuth, async (req, res) => { // ─── GET /api/docker/containers ─────────────────────────────────────────────── -router.get('/api/docker/containers', requireAuth, (req, res) => { +router.get('/api/docker/containers', requireAuth, requirePermission('metrics.view'), (req, res) => { try { const raw = safeExec('docker ps -a --format "{{.Names}}|{{.Image}}|{{.State}}|{{.Status}}|{{.Ports}}" 2>/dev/null', 10000); if (!raw) { @@ -278,7 +278,7 @@ router.post('/api/system/exec', requireAuth, requirePermission('server.config'), // ─── GET /api/speed-test ────────────────────────────────────────────────────── -router.get('/api/speed-test', requireAuth, (req, res) => { +router.get('/api/speed-test', requireAuth, requirePermission('metrics.view'), (req, res) => { const size = Math.min(parseInt(req.query.size, 10) || 1048576, 10485760); // max 10MB res.set({ 'Content-Type': 'application/octet-stream', diff --git a/web-nodejs/server.js b/web-nodejs/server.js index c67e7f35..67a32573 100644 --- a/web-nodejs/server.js +++ b/web-nodejs/server.js @@ -18,7 +18,7 @@ const https = require('https'); const config = require('./config/config'); const securityMiddleware = require('./middleware/security'); const { initI18n } = require('./middleware/i18n'); -const { apiLimiter } = require('./middleware/rateLimiter'); +const { apiLimiter, widgetLimiter } = require('./middleware/rateLimiter'); const { csrfTokenProvider, doubleCsrfProtection, downgradeToHttp: csrfDowngradeToHttp } = require('./middleware/csrf'); const { roleHasPermission, isSuperAdminRole } = require('./middleware/auth'); const authService = require('./services/authService'); @@ -138,7 +138,14 @@ app.use('/wallpapers', express.static(path.join(__dirname, 'wallpapers'), { immutable: true })); -// Rate limiting for API +// Rate limiting for API. +// SECURITY (audit fix M-03, 2026-04-10): high-frequency widget refresh paths +// have their own higher-quota limiter mounted BEFORE the general one so they +// are still bounded but do not eat into the regular API budget. +const widgetPaths = ['/api/stats', '/api/server/status', '/api/devices', '/api/audit/conn']; +for (const p of widgetPaths) { + app.use(p, widgetLimiter); +} app.use('/api/', apiLimiter); // RustDesk Client API — mounted BEFORE CSRF because desktop clients use Bearer @@ -746,6 +753,34 @@ function printStartupBanner(protocol, port) { console.log(' Ensure a trusted reverse proxy sets X-Forwarded-For correctly.'); console.log(''); } + + // L-01 (audit 2026-04-10): warn about disabled proxy trust in production + // — rate limiters and audit logs will see the proxy IP, not the client IP. + if (process.env.NODE_ENV === 'production' && (!trustProxy || trustProxy === false || trustProxy === 0)) { + console.log(' ⚠️ WARNING [SECURITY]: NODE_ENV=production but TRUST_PROXY is disabled.'); + console.log(' If the panel is behind a reverse proxy (nginx, Cloudflare, ALB,'); + console.log(' Traefik…) rate-limit keys and audit logs will record the proxy IP,'); + console.log(' not the real client IP. Set TRUST_PROXY=1 (single proxy) or a CIDR list.'); + console.log(''); + } + + // H-04 (audit 2026-04-10): unconditional banner when the RustDesk client + // API TOTP bypass is enabled, regardless of acknowledgement — the bypass + // weakens 2FA on the WAN-facing :21121 endpoint and operators MUST be + // aware of it on every restart. + if (config.rustdeskApiDisableTotp) { + if (!config.rustdeskApiDisableTotpAck) { + console.log(' ⛔ ERROR [SECURITY]: RUSTDESK_API_DISABLE_TOTP=true but ACK flag is missing.'); + console.log(' The bypass is IGNORED. Set RUSTDESK_API_DISABLE_TOTP_ACKNOWLEDGED=true'); + console.log(' to confirm you accept disabling 2FA on the RustDesk client login.'); + console.log(''); + } else { + console.log(' ⚠️ WARNING [SECURITY]: TOTP is DISABLED on the RustDesk client API (:21121).'); + console.log(' RustDesk desktop clients can log in with username+password only.'); + console.log(' The web panel still enforces 2FA independently.'); + console.log(''); + } + } } function redactUrlForLog(rawUrl) {