fix(security): harden server against verified code-review findings

Applies fixes for 20 adversarially-verified findings from a whole-codebase
security review (server side). All Go build-tag variants build, `go vet` is
clean, and the suite passes (the sole failing test, ws TestEmitEvents, is a
pre-existing nil-harness failure unrelated to these changes).

High severity:
- auth: close TOCTOU in TOTP verify rate-limit by recording each attempt
  atomically up-front (was Check-then-Allow), restoring the per-user
  brute-force cap.
- plugin: enforce the CPU/time budget on every WASM guest call via a
  WithTimeout context (WithCloseOnContextDone interrupts runaways); the
  configured budget was previously parsed but never applied.
- api/waf: inspect request bodies for chunked (ContentLength==-1) requests
  so the SQLi/XSS/RCE body rules can no longer be bypassed.
- ws: rate-limit voice_join/voice_leave and voice_e2ee announce/offer, which
  fan out to every participant and could force mass disconnects.

Medium severity:
- api: run bcrypt on the unknown-user login path (no || short-circuit) to
  remove the timing-based username-enumeration oracle.
- ws: verify LiveKit webhooks via the SDK receiver so the signature is bound
  to the body hash (kills forgery/replay).
- authz: require READ_MESSAGES for reactions and for plugin-command
  broadcasts; route the latter through RequireChannelAccess.
- api: cache the client-update signature fetch and rate-limit the endpoint.
- service: propagate DeleteOtherSessions failure from ChangePassword instead
  of silently reporting success.
- api: trust the rightmost non-proxy X-Forwarded-For entry, not the
  client-controllable leftmost one.
- plugin: route auto-registered commands through the conflict-checked
  RegisterCommand; pin the DNS-validated IP for host_http dials
  (DNS-rebinding TOCTOU).
- api: mark access-controlled downloads private/no-cache + Vary: Origin.

Low severity:
- auth: fail closed when a fully-shaped TOTP ciphertext fails GCM auth
  (was returning the ciphertext as plaintext).
- api: apply the livekit-proxy path allowlist to WebSocket upgrades too.
- service: verify attachment ownership before linking (IDOR).
- admin: bound the bootstrap setup invite (5 uses / 24h); re-verify the
  update binary hash immediately before rename+spawn (TOCTOU).
- service: require BanMembers + role hierarchy for moderation ban/unban.

chore: stop tracking the stray Server/owncord-server.exe build artifact.

Test infra: add uploader_id to the hand-rolled ws test attachment schemas and
make MemStore.GetAttachmentByID a no-op lookup, matching production/DB behavior.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
J3vb
2026-07-17 21:08:54 +02:00
co-authored by Claude Opus 4.8
parent 34a84bcd5d
commit 7b178ff30b
30 changed files with 449 additions and 104 deletions
+4 -1
View File
@@ -142,7 +142,10 @@ func handleSetup(database *db.DB, limiter *auth.RateLimiter, allowedOrigins []st
_, _ = database.CreateChannel("General", "voice", "Voice Channels", "", 0) _, _ = database.CreateChannel("General", "voice", "Voice Channels", "", 0)
// Generate a bootstrap invite code so the owner can invite others. // Generate a bootstrap invite code so the owner can invite others.
inviteCode, err := database.CreateInvite(uid, 0, nil) // unlimited uses, no expiry // Bound it (5 uses / 24h) rather than minting an unlimited, non-expiring
// invite — the owner can create fresh invites once logged in.
bootstrapInviteExpiry := time.Now().Add(24 * time.Hour)
inviteCode, err := database.CreateInvite(uid, 5, &bootstrapInviteExpiry)
if err != nil { if err != nil {
writeErr(w, http.StatusInternalServerError, "INTERNAL_ERROR", "failed to generate invite code") writeErr(w, http.StatusInternalServerError, "INTERNAL_ERROR", "failed to generate invite code")
return return
+39
View File
@@ -2,6 +2,9 @@ package admin
import ( import (
"context" "context"
"crypto/sha256"
"encoding/hex"
"io"
"log/slog" "log/slog"
"net/http" "net/http"
"os" "os"
@@ -84,6 +87,17 @@ func handleApplyUpdate(u *updater.Updater, hub HubBroadcaster, _ string) http.Ha
return return
} }
// Snapshot the hash of the just-verified staged binary. It is re-checked
// immediately before rename+spawn to close the TOCTOU window between
// verification here and the swap in the background goroutine below.
stagedHash, err := fileSHA256(newPath)
if err != nil {
slog.Error("update: failed to hash staged binary", "err", err)
_ = os.Remove(newPath)
writeErr(w, http.StatusInternalServerError, "INTERNAL_ERROR", "failed to stage update")
return
}
// Respond to the client before shutting down. // Respond to the client before shutting down.
writeJSON(w, http.StatusOK, map[string]string{ writeJSON(w, http.StatusOK, map[string]string{
"status": "applying", "status": "applying",
@@ -97,6 +111,14 @@ func handleApplyUpdate(u *updater.Updater, hub HubBroadcaster, _ string) http.Ha
} }
time.Sleep(5 * time.Second) time.Sleep(5 * time.Second)
// TOCTOU guard: re-verify the staged binary is byte-for-byte the one
// we verified before responding. If it was swapped between then and
// now, abort without renaming or spawning it.
if err := u.VerifyChecksum(newPath, stagedHash); err != nil {
slog.Error("update: staged binary re-verification failed, aborting update", "error", err)
return
}
// Rename: current -> .old, .new -> current // Rename: current -> .old, .new -> current
_ = os.Remove(oldPath) // remove any stale .old _ = os.Remove(oldPath) // remove any stale .old
if err := os.Rename(exePath, oldPath); err != nil { if err := os.Rename(exePath, oldPath); err != nil {
@@ -137,3 +159,20 @@ func handleApplyUpdate(u *updater.Updater, hub HubBroadcaster, _ string) http.Ha
}() }()
}) })
} }
// fileSHA256 returns the hex-encoded SHA256 of the file at path. Used to
// snapshot a verified update binary so it can be re-checked (via
// updater.VerifyChecksum) immediately before it is renamed and executed.
func fileSHA256(path string) (string, error) {
f, err := os.Open(path)
if err != nil {
return "", err
}
defer f.Close() //nolint:errcheck
h := sha256.New()
if _, err := io.Copy(h, f); err != nil {
return "", err
}
return hex.EncodeToString(h.Sum(nil)), nil
}
+11 -1
View File
@@ -317,7 +317,17 @@ func handleLogin(database *db.DB, limiter *auth.RateLimiter, partialStore *auth.
failKey := "login_fail:" + ip failKey := "login_fail:" + ip
userFailKey := "login_user_fail:" + req.Username userFailKey := "login_user_fail:" + req.Username
if user == nil || !auth.CheckPassword(user.PasswordHash, req.Password) { // Always run the password check — with an empty hash when the user does
// not exist. auth.CheckPassword performs a dummy bcrypt comparison for an
// empty hash, so bcrypt executes on every path and response time stays
// constant, preventing timing-based username enumeration. (A `user == nil
// || CheckPassword(...)` short-circuit would skip bcrypt entirely for
// unknown usernames, reintroducing the timing side-channel.)
storedHash := ""
if user != nil {
storedHash = user.PasswordHash
}
if !auth.CheckPassword(storedHash, req.Password) {
// Track failures per-IP; lockout on threshold. // Track failures per-IP; lockout on threshold.
if !limiter.Allow(failKey, loginFailureThreshold, loginFailureWindow) { if !limiter.Allow(failKey, loginFailureThreshold, loginFailureWindow) {
limiter.Lockout(lockKey, loginLockoutDuration) limiter.Lockout(lockKey, loginLockoutDuration)
+4 -2
View File
@@ -67,8 +67,10 @@ func handleClientUpdate(u *updater.Updater) http.HandlerFunc {
return return
} }
// Fetch the signature file content (small text file). // Fetch the signature file content (small text file). Cached with the
sigContent, err := u.FetchTextAsset(r.Context(), sigURL) // same TTL as the release info so this unauthenticated endpoint does not
// perform an outbound fetch on every request (DoS hardening).
sigContent, err := u.FetchTextAssetCached(r.Context(), sigURL)
if err != nil { if err != nil {
http.Error(w, "failed to fetch signature", http.StatusBadGateway) http.Error(w, "failed to fetch signature", http.StatusBadGateway)
return return
+3
View File
@@ -31,6 +31,9 @@ const (
// livekitProxyRateLimitPerMinute is the maximum LiveKit proxy requests per IP per minute. // livekitProxyRateLimitPerMinute is the maximum LiveKit proxy requests per IP per minute.
livekitProxyRateLimitPerMinute = 30 livekitProxyRateLimitPerMinute = 30
// clientUpdateRateLimitPerMinute is the maximum client-update checks per IP per minute.
clientUpdateRateLimitPerMinute = 30
// loginFailureThreshold is the number of failed login attempts (within // loginFailureThreshold is the number of failed login attempts (within
// loginFailureWindow) before the IP is locked out. // loginFailureWindow) before the IP is locked out.
loginFailureThreshold = 9 loginFailureThreshold = 9
+10 -6
View File
@@ -59,11 +59,9 @@ func NewLiveKitProxy(livekitURL string, allowedOrigins []string) http.Handler {
blockedSegments := map[string]bool{"admin": true, "metrics": true, "debug": true, "twirp": true} blockedSegments := map[string]bool{"admin": true, "metrics": true, "debug": true, "twirp": true}
return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
// Detect WebSocket upgrade requests. // Enforce the path allowlist and Origin check for EVERY request,
if isWebSocketUpgrade(r) { // including WebSocket upgrades — otherwise a client could reach a
proxyWebSocket(w, r, &wsTarget, allowedOrigins) // blocked/admin endpoint simply by sending an Upgrade header.
return
}
// Block sensitive LiveKit endpoints (exact segment match). // Block sensitive LiveKit endpoints (exact segment match).
for _, seg := range strings.Split(strings.ToLower(r.URL.Path), "/") { for _, seg := range strings.Split(strings.ToLower(r.URL.Path), "/") {
@@ -76,7 +74,7 @@ func NewLiveKitProxy(livekitURL string, allowedOrigins []string) http.Handler {
} }
} }
// Validate Origin header for HTTP requests (mirrors WS OriginPatterns). // Validate Origin header (mirrors WS OriginPatterns).
if !isOriginAllowed(r, allowedOrigins) { if !isOriginAllowed(r, allowedOrigins) {
writeJSON(w, http.StatusForbidden, errorResponse{ writeJSON(w, http.StatusForbidden, errorResponse{
Error: "FORBIDDEN", Error: "FORBIDDEN",
@@ -85,6 +83,12 @@ func NewLiveKitProxy(livekitURL string, allowedOrigins []string) http.Handler {
return return
} }
// Detect WebSocket upgrade requests.
if isWebSocketUpgrade(r) {
proxyWebSocket(w, r, &wsTarget, allowedOrigins)
return
}
httpProxy.ServeHTTP(w, r) httpProxy.ServeHTTP(w, r)
}) })
} }
+15 -5
View File
@@ -218,13 +218,23 @@ func clientIPWithProxies(r *http.Request, trustedCIDRs []string) string {
} }
} }
// Fall back to the leftmost (client) entry in X-Forwarded-For. // Fall back to X-Forwarded-For, walking from the RIGHT and skipping entries
// that are themselves trusted proxies. The first non-trusted, valid address
// is the real client. Taking the leftmost entry (BUG-112) would trust a
// client-supplied value: a client can prepend a spoofed IP
// (`X-Forwarded-For: <spoofed>, <real>`) that the proxy then appends to,
// letting it forge per-IP rate-limit and lockout keys.
if xff := r.Header.Get("X-Forwarded-For"); xff != "" { if xff := r.Header.Get("X-Forwarded-For"); xff != "" {
parts := strings.SplitN(xff, ",", 2) parts := strings.Split(xff, ",")
if client := strings.TrimSpace(parts[0]); client != "" { for i := len(parts) - 1; i >= 0; i-- {
if net.ParseIP(client) != nil { candidate := strings.TrimSpace(parts[i])
return client if candidate == "" || net.ParseIP(candidate) == nil {
continue
} }
if trusted, _ := isTrustedProxy(candidate, trustedCIDRs); trusted {
continue // our own proxy hop, keep walking left
}
return candidate
} }
} }
+6 -2
View File
@@ -251,8 +251,12 @@ func NewRouter(cfg *config.Config, database *db.DB, ver string, logBuf *admin.Ri
}) })
}) })
// Client auto-update endpoint (unauthenticated). // Client auto-update endpoint (unauthenticated). Per-IP rate limited to
MountClientUpdateRoute(r, u) // bound abuse; the signature fetch is cached inside the updater (DoS fix).
MountClientUpdateRoute(
r.With(RateLimitMiddleware(limiter, clientUpdateRateLimitPerMinute, time.Minute, cfg.Server.TrustedProxies)),
u,
)
// Issue 15: Warn if AllowedOrigins contains wildcard. // Issue 15: Warn if AllowedOrigins contains wildcard.
for _, o := range cfg.Server.AllowedOrigins { for _, o := range cfg.Server.AllowedOrigins {
+10 -2
View File
@@ -66,7 +66,14 @@ func handleVerifyTOTP(database *db.DB, partialStore *auth.PartialAuthStore, limi
} }
totpRateLimitKey := fmt.Sprintf("totp_fail:%d", challenge.UserID) totpRateLimitKey := fmt.Sprintf("totp_fail:%d", challenge.UserID)
if !limiter.Check(totpRateLimitKey, totpFailureRateLimit, totpFailureWindow) { // Atomically record this attempt and reject once the per-user failure cap
// is reached. Recording up-front — rather than a read-only Check now and
// Allow only on failure — closes a TOCTOU where many concurrent requests
// reusing one valid partial token all pass the read-only check before any
// failure is recorded, defeating the per-user brute-force cap (the only
// cross-IP defence). A successful verification resets the counter below,
// so legitimate retries are not penalised.
if !limiter.Allow(totpRateLimitKey, totpFailureRateLimit, totpFailureWindow) {
writeJSON(w, http.StatusTooManyRequests, errorResponse{ writeJSON(w, http.StatusTooManyRequests, errorResponse{
Error: "RATE_LIMITED", Error: "RATE_LIMITED",
Message: "too many failed attempts, try again later", Message: "too many failed attempts, try again later",
@@ -94,7 +101,8 @@ func handleVerifyTOTP(database *db.DB, partialStore *auth.PartialAuthStore, limi
} }
if !auth.VerifyTOTPCodeOnce(secret, strings.TrimSpace(req.Code), time.Now().UTC(), user.ID, usedTOTPCodes) { if !auth.VerifyTOTPCodeOnce(secret, strings.TrimSpace(req.Code), time.Now().UTC(), user.ID, usedTOTPCodes) {
limiter.Allow(totpRateLimitKey, totpFailureRateLimit, totpFailureWindow) // The attempt was already recorded atomically up-front via
// limiter.Allow; only the per-partial-token counter is advanced here.
partialStore.RegisterFailure(partialToken, partialAuthMaxFailures) partialStore.RegisterFailure(partialToken, partialAuthMaxFailures)
writeJSON(w, http.StatusUnauthorized, errorResponse{ writeJSON(w, http.StatusUnauthorized, errorResponse{
Error: "UNAUTHORIZED", Error: "UNAUTHORIZED",
+6 -1
View File
@@ -304,7 +304,12 @@ func handleServeFile(database *db.DB, store *storage.Storage, allowedOrigins []s
disposition = "attachment" disposition = "attachment"
} }
w.Header().Set("Content-Disposition", mime.FormatMediaType(disposition, map[string]string{"filename": aa.Filename})) w.Header().Set("Content-Disposition", mime.FormatMediaType(disposition, map[string]string{"filename": aa.Filename}))
w.Header().Set("Cache-Control", fmt.Sprintf("public, max-age=%d, immutable", fileCacheMaxAgeSeconds)) // These downloads are access-controlled, so they must never be stored by
// shared/proxy caches (info-leak). Mark private and force revalidation.
w.Header().Set("Cache-Control", fmt.Sprintf("private, max-age=%d, no-cache", fileCacheMaxAgeSeconds))
// The Access-Control-Allow-Origin header below reflects the request
// Origin, so responses vary by Origin and must not be cross-served.
w.Header().Set("Vary", "Origin")
// CORS: allow webview to read the response body using configured origins. // CORS: allow webview to read the response body using configured origins.
if origin := r.Header.Get("Origin"); origin != "" { if origin := r.Header.Get("Origin"); origin != "" {
for _, allowed := range allowedOrigins { for _, allowed := range allowedOrigins {
+4 -3
View File
@@ -727,10 +727,11 @@ func TestServeFile_Success(t *testing.T) {
t.Error("expected Content-Type header on served file") t.Error("expected Content-Type header on served file")
} }
// Verify cache control header. // Verify cache control header. Access-controlled downloads must be marked
// private + no-cache so shared/proxy caches never store them (info-leak).
cc := rr2.Header().Get("Cache-Control") cc := rr2.Header().Get("Cache-Control")
if cc != "public, max-age=31536000, immutable" { if cc != "private, max-age=31536000, no-cache" {
t.Errorf("Cache-Control = %q, want 'public, max-age=31536000, immutable'", cc) t.Errorf("Cache-Control = %q, want 'private, max-age=31536000, no-cache'", cc)
} }
// Verify Content-Disposition header. // Verify Content-Disposition header.
+6 -2
View File
@@ -92,8 +92,12 @@ func NewWAFMiddleware(paranoiaLevel int) func(http.Handler) http.Handler {
return return
} }
// Process request body (if applicable) // Process request body (if applicable). Use ContentLength != 0 so
if r.Body != nil && r.ContentLength > 0 { // chunked requests (Transfer-Encoding: chunked → ContentLength == -1)
// are inspected too; otherwise the SQLi/XSS/RCE body rules are silently
// skipped for them. The read is bounded by SecRequestBodyLimit inside
// Coraza. ContentLength == 0 (no body) still skips inspection.
if r.Body != nil && r.ContentLength != 0 {
if it, _, err := tx.ReadRequestBodyFrom(r.Body); it != nil { if it, _, err := tx.ReadRequestBodyFrom(r.Body); it != nil {
handleWAFInterruption(w, it) handleWAFInterruption(w, it)
return return
+8 -5
View File
@@ -138,11 +138,14 @@ func DecryptTOTPSecret(key []byte, ciphertext string) (string, error) {
nonce, sealed := data[:nonceSize], data[nonceSize:] nonce, sealed := data[:nonceSize], data[nonceSize:]
plaintext, err := gcm.Open(nil, nonce, sealed, nil) plaintext, err := gcm.Open(nil, nonce, sealed, nil)
if err != nil { if err != nil {
// Decryption failed -- likely an unencrypted legacy secret or wrong key. // The value has the full encrypted shape (valid hex, long enough for
// Return as-is for backwards compatibility. // nonce+tag) but GCM authentication failed. That is a real error — a
slog.Warn("TOTP secret decryption failed — returning as plaintext (check TOTP_ENCRYPTION_KEY)", // wrong TOTP_ENCRYPTION_KEY or a tampered/corrupted ciphertext — not a
"error", err) // legacy plaintext secret (those are caught by the not-hex and
return ciphertext, nil //nolint:nilerr // too-short branches above). Fail CLOSED: returning the ciphertext as
// if it were the secret would silently mask key misconfiguration.
slog.Error("TOTP secret decryption failed — check TOTP_ENCRYPTION_KEY", "error", err)
return "", fmt.Errorf("decrypting TOTP secret: %w", err)
} }
return string(plaintext), nil return string(plaintext), nil
+2
View File
@@ -69,6 +69,8 @@ require (
github.com/gotnospirit/makeplural v0.0.0-20180622080156-a5f48d94d976 // indirect github.com/gotnospirit/makeplural v0.0.0-20180622080156-a5f48d94d976 // indirect
github.com/gotnospirit/messageformat v0.0.0-20221001023931-dfe49f1eb092 // indirect github.com/gotnospirit/messageformat v0.0.0-20221001023931-dfe49f1eb092 // indirect
github.com/grpc-ecosystem/grpc-gateway/v2 v2.28.0 // indirect github.com/grpc-ecosystem/grpc-gateway/v2 v2.28.0 // indirect
github.com/hashicorp/go-cleanhttp v0.5.2 // indirect
github.com/hashicorp/go-retryablehttp v0.7.7 // indirect
github.com/jackc/pgpassfile v1.0.0 // indirect github.com/jackc/pgpassfile v1.0.0 // indirect
github.com/jackc/pgservicefile v0.0.0-20240606120523-5a60cdf6a761 // indirect github.com/jackc/pgservicefile v0.0.0-20240606120523-5a60cdf6a761 // indirect
github.com/jackc/puddle/v2 v2.2.2 // indirect github.com/jackc/puddle/v2 v2.2.2 // indirect
+4
View File
@@ -119,6 +119,10 @@ github.com/gotnospirit/messageformat v0.0.0-20221001023931-dfe49f1eb092 h1:c7gcN
github.com/gotnospirit/messageformat v0.0.0-20221001023931-dfe49f1eb092/go.mod h1:ZZAN4fkkful3l1lpJwF8JbW41ZiG9TwJ2ZlqzQovBNU= github.com/gotnospirit/messageformat v0.0.0-20221001023931-dfe49f1eb092/go.mod h1:ZZAN4fkkful3l1lpJwF8JbW41ZiG9TwJ2ZlqzQovBNU=
github.com/grpc-ecosystem/grpc-gateway/v2 v2.28.0 h1:HWRh5R2+9EifMyIHV7ZV+MIZqgz+PMpZ14Jynv3O2Zs= github.com/grpc-ecosystem/grpc-gateway/v2 v2.28.0 h1:HWRh5R2+9EifMyIHV7ZV+MIZqgz+PMpZ14Jynv3O2Zs=
github.com/grpc-ecosystem/grpc-gateway/v2 v2.28.0/go.mod h1:JfhWUomR1baixubs02l85lZYYOm7LV6om4ceouMv45c= github.com/grpc-ecosystem/grpc-gateway/v2 v2.28.0/go.mod h1:JfhWUomR1baixubs02l85lZYYOm7LV6om4ceouMv45c=
github.com/hashicorp/go-cleanhttp v0.5.2 h1:035FKYIWjmULyFRBKPs8TBQoi0x6d9G4xc9neXJWAZQ=
github.com/hashicorp/go-cleanhttp v0.5.2/go.mod h1:kO/YDlP8L1346E6Sodw+PrpBSV4/SoxCXGY6BqNFT48=
github.com/hashicorp/go-retryablehttp v0.7.7 h1:C8hUCYzor8PIfXHa4UrZkU4VvK8o9ISHxT2Q8+VepXU=
github.com/hashicorp/go-retryablehttp v0.7.7/go.mod h1:pkQpWZeYWskR+D1tR2O5OcBFOxfA7DoAO6xtkuQnHTk=
github.com/hashicorp/golang-lru/v2 v2.0.7 h1:a+bsQ5rvGLjzHuww6tVxozPZFVghXaHOwFs4luLUK2k= github.com/hashicorp/golang-lru/v2 v2.0.7 h1:a+bsQ5rvGLjzHuww6tVxozPZFVghXaHOwFs4luLUK2k=
github.com/hashicorp/golang-lru/v2 v2.0.7/go.mod h1:QeFd9opnmA6QUJc5vARoKUSoFhyfM2/ZepoAG6RGpeM= github.com/hashicorp/golang-lru/v2 v2.0.7/go.mod h1:QeFd9opnmA6QUJc5vARoKUSoFhyfM2/ZepoAG6RGpeM=
github.com/jackc/pgpassfile v1.0.0 h1:/6Hmqy13Ss2zCq62VdNG8tM1wchn8zjSGOBJ6icpsIM= github.com/jackc/pgpassfile v1.0.0 h1:/6Hmqy13Ss2zCq62VdNG8tM1wchn8zjSGOBJ6icpsIM=
+28 -11
View File
@@ -76,26 +76,43 @@ func (r *Registry) HTTPDo(ctx context.Context, inst *Instance, req HTTPRequest)
for k, v := range req.Header { for k, v := range req.Header {
httpReq.Header.Set(k, v) httpReq.Header.Set(k, v)
} }
// Custom transport with a guarded DialContext: every actual TCP dial // Custom transport with a guarded DialContext: the host is resolved once,
// re-checks the resolved IP, closing the DNS-rebinding TOCTOU window // every candidate IP is validated against the blocklist, and the actual
// between rejectPrivateAddrs above and the underlying dial. // connection is made to that specific vetted IP — never re-resolved by
// hostname. This closes the DNS-rebinding TOCTOU window where a second
// lookup (the one net.Dialer would perform on a hostname) could return an
// internal IP after rejectPrivateAddrs above had already approved the name.
dialer := &net.Dialer{Timeout: httpTimeout} dialer := &net.Dialer{Timeout: httpTimeout}
transport := &http.Transport{ transport := &http.Transport{
DialContext: func(ctx context.Context, network, addr string) (net.Conn, error) { DialContext: func(ctx context.Context, network, addr string) (net.Conn, error) {
h, _, splitErr := net.SplitHostPort(addr) h, port, splitErr := net.SplitHostPort(addr)
if splitErr != nil { if splitErr != nil {
return nil, splitErr return nil, splitErr
} }
ip := net.ParseIP(h) // IP literal: validate and dial as-is (no resolution happens).
if ip == nil { if ip := net.ParseIP(h); ip != nil {
// Hostname — resolve and validate every address before dial. if err := ipAllowed(ip); err != nil {
if err := rejectPrivateAddrs(ctx, h); err != nil {
return nil, fmt.Errorf("%w: %v", ErrHTTPHostDenied, err) return nil, fmt.Errorf("%w: %v", ErrHTTPHostDenied, err)
} }
} else if err := ipAllowed(ip); err != nil { return dialer.DialContext(ctx, network, addr)
return nil, fmt.Errorf("%w: %v", ErrHTTPHostDenied, err)
} }
return dialer.DialContext(ctx, network, addr) // Hostname: resolve once, validate every returned address, then
// dial the concrete vetted IP so the connection target is exactly
// the address that was checked.
resolver := &net.Resolver{}
ips, lookupErr := resolver.LookupIPAddr(ctx, h)
if lookupErr != nil {
return nil, fmt.Errorf("%w: dns lookup failed: %v", ErrHTTPHostDenied, lookupErr)
}
if len(ips) == 0 {
return nil, fmt.Errorf("%w: no addresses for %s", ErrHTTPHostDenied, h)
}
for _, resolved := range ips {
if err := ipAllowed(resolved.IP); err != nil {
return nil, fmt.Errorf("%w: %v", ErrHTTPHostDenied, err)
}
}
return dialer.DialContext(ctx, network, net.JoinHostPort(ips[0].IP.String(), port))
}, },
} }
client := &http.Client{ client := &http.Client{
+43 -9
View File
@@ -5,7 +5,7 @@
// elsewhere in the repo so the default sqlite-only build does not pull // elsewhere in the repo so the default sqlite-only build does not pull
// wazero into go.mod at runtime. // wazero into go.mod at runtime.
// //
// Architecture // # Architecture
// //
// The wazero-tagged build provides: // The wazero-tagged build provides:
// //
@@ -36,8 +36,10 @@ import (
"encoding/json" "encoding/json"
"fmt" "fmt"
"io" "io"
"log/slog"
"os" "os"
"strings" "strings"
"time"
"github.com/tetratelabs/wazero" "github.com/tetratelabs/wazero"
"github.com/tetratelabs/wazero/api" "github.com/tetratelabs/wazero/api"
@@ -121,17 +123,26 @@ func (r *Registry) activateWithRuntime(ctx context.Context, platform any, inst *
return fmt.Errorf("plugin %q: instantiate: %w", inst.Manifest.Name, err) return fmt.Errorf("plugin %q: instantiate: %w", inst.Manifest.Name, err)
} }
// Register the module and auto-bind any commands the plugin exports // Store the module under the lock, then auto-bind any commands the plugin
// via list_commands. The plugin must also have declared the `commands` // exports via list_commands. Binding is routed through RegisterCommand so
// capability in its manifest, otherwise no binding happens. // each name goes through the same normalization (trim "/" + lowercase) and
// conflict check the direct registration path uses: a command already owned
// by a DIFFERENT plugin is refused rather than silently clobbered, closing
// the cross-plugin command-hijack hole. RegisterCommand acquires r.mu
// itself, so it is called outside the lock below to avoid re-entrant
// locking. The plugin must also have declared the `commands` capability in
// its manifest, otherwise no binding happens.
r.mu.Lock() r.mu.Lock()
inst.module = module inst.module = module
r.mu.Unlock()
if inst.Manifest.HasCapability(CapCommands) { if inst.Manifest.HasCapability(CapCommands) {
for _, cmd := range listExportedCommands(ctx, module) { for _, cmd := range listExportedCommands(ctx, module) {
r.commands[cmd] = inst if err := r.RegisterCommand(cmd, inst); err != nil {
slog.Warn("plugin: skipping command binding",
"plugin", inst.Manifest.Name, "command", cmd, "err", err)
}
} }
} }
r.mu.Unlock()
return nil return nil
} }
@@ -195,9 +206,27 @@ func (r *Registry) invokeCommand(ctx context.Context, inst *Instance, userID, ch
return &CommandResult{Reply: fmt.Sprintf("plugin %s: marshal payload: %v", inst.Manifest.Name, err)}, true return &CommandResult{Reply: fmt.Sprintf("plugin %s: marshal payload: %v", inst.Manifest.Name, err)}, true
} }
// Enforce the plugin's CPU budget. The effective budget is the manifest's
// Resources.CPUBudgetMs, falling back to the configured default, then a
// hard 100ms floor so a zero/negative value can never mean "no limit".
// Every guest call (allocate / command_dispatch / deallocate) runs under
// this deadline instead of the long-lived WebSocket context. The runtime
// was created WithCloseOnContextDone(true), so an expired deadline closes
// the module and interrupts a runaway guest (e.g. `for {}`) — the Call
// returns an error rather than panicking, which the paths below surface.
budgetMs := inst.Manifest.Resources.CPUBudgetMs
if budgetMs <= 0 {
budgetMs = r.cfg.CPUBudgetMs
}
if budgetMs <= 0 {
budgetMs = 100
}
callCtx, cancel := context.WithTimeout(ctx, time.Duration(budgetMs)*time.Millisecond)
defer cancel()
// Allocate guest memory for the input payload. // Allocate guest memory for the input payload.
size := uint64(len(payload)) size := uint64(len(payload))
ptrs, callErr := allocFn.Call(ctx, size) ptrs, callErr := allocFn.Call(callCtx, size)
if callErr != nil || len(ptrs) == 0 { if callErr != nil || len(ptrs) == 0 {
return &CommandResult{Reply: fmt.Sprintf("plugin %s: allocate(%d): %v", inst.Manifest.Name, size, callErr)}, true return &CommandResult{Reply: fmt.Sprintf("plugin %s: allocate(%d): %v", inst.Manifest.Name, size, callErr)}, true
} }
@@ -208,14 +237,19 @@ func (r *Registry) invokeCommand(ctx context.Context, inst *Instance, userID, ch
return &CommandResult{Reply: fmt.Sprintf("plugin %s: memory write at %d failed", inst.Manifest.Name, ptr)}, true return &CommandResult{Reply: fmt.Sprintf("plugin %s: memory write at %d failed", inst.Manifest.Name, ptr)}, true
} }
results, callErr := dispatchFn.Call(ctx, ptr, size) results, callErr := dispatchFn.Call(callCtx, ptr, size)
// Free the input buffer regardless of dispatch outcome. // Free the input buffer regardless of dispatch outcome.
if deallocFn != nil { if deallocFn != nil {
_, _ = deallocFn.Call(ctx, ptr, size) _, _ = deallocFn.Call(callCtx, ptr, size)
} }
if callErr != nil { if callErr != nil {
// Surface a CPU-budget overrun as a clean, specific error rather than
// leaking the raw "module closed with context deadline exceeded".
if callCtx.Err() == context.DeadlineExceeded {
return &CommandResult{Reply: fmt.Sprintf("plugin %s: command exceeded CPU budget of %dms", inst.Manifest.Name, budgetMs)}, true
}
return &CommandResult{Reply: fmt.Sprintf("plugin %s: dispatch: %v", inst.Manifest.Name, callErr)}, true return &CommandResult{Reply: fmt.Sprintf("plugin %s: dispatch: %v", inst.Manifest.Name, callErr)}, true
} }
if len(results) < 2 { if len(results) < 2 {
+24 -1
View File
@@ -173,6 +173,26 @@ func (s *MessageService) SendMessage(ctx context.Context, p SendMessageParams) (
} }
} }
// Verify attachment ownership before persisting, to prevent hijacking
// another user's unlinked upload (IDOR). Any referenced attachment that
// exists must belong to the sender (uploader_id == p.UserID) and must not
// already be linked to a message (message_id IS NULL). Nonexistent IDs are
// ignored — LinkAttachmentsToMessage silently skips them. Checked before
// CreateMessage so a failed ownership check never persists a message.
for _, aid := range p.AttachmentIDs {
att, attErr := s.st.GetAttachmentByID(aid)
if attErr != nil {
slog.Error("MessageService.SendMessage GetAttachmentByID", "err", attErr, "attachment_id", aid)
return nil, fmt.Errorf("%w: failed to verify attachment ownership", ErrInternal)
}
if att == nil {
continue // nonexistent — the link query will skip it
}
if att.UploaderID == nil || *att.UploaderID != p.UserID || att.MessageID != nil {
return nil, fmt.Errorf("%w: attachment not owned by sender or already linked", ErrForbidden)
}
}
// Persist message. // Persist message.
msgID, err := s.st.CreateMessage(p.ChannelID, p.UserID, content, p.ReplyTo) msgID, err := s.st.CreateMessage(p.ChannelID, p.UserID, content, p.ReplyTo)
if err != nil { if err != nil {
@@ -435,7 +455,10 @@ func (s *MessageService) handleReaction(userID, msgID int64, emoji string, add b
if dmErr != nil || !ok { if dmErr != nil || !ok {
return nil, fmt.Errorf("%w: not a DM participant", ErrBadRequest) return nil, fmt.Errorf("%w: not a DM participant", ErrBadRequest)
} }
} else if !s.perms.HasChannelPerm(userID, msg.ChannelID, permissions.AddReactions) { } else if !s.perms.HasChannelPerm(userID, msg.ChannelID, permissions.ReadMessages|permissions.AddReactions) {
// Require READ_MESSAGES in addition to ADD_REACTIONS so a user cannot
// react in a channel they cannot read. Mirrors checkSendPermission,
// which requires ReadMessages|SendMessages for non-DM sends.
return nil, fmt.Errorf("%w: missing ADD_REACTIONS permission", ErrForbidden) return nil, fmt.Errorf("%w: missing ADD_REACTIONS permission", ErrForbidden)
} }
+49 -3
View File
@@ -6,18 +6,54 @@ import (
"log/slog" "log/slog"
"time" "time"
"github.com/owncord/server/permissions"
"github.com/owncord/server/store" "github.com/owncord/server/store"
"github.com/owncord/server/telemetry" "github.com/owncord/server/telemetry"
) )
// ModerationService handles user ban/unban operations. // ModerationService handles user ban/unban operations.
type ModerationService struct { type ModerationService struct {
st store.Store st store.Store
perms *PermissionService
} }
// NewModerationService creates a ModerationService. // NewModerationService creates a ModerationService.
func NewModerationService(st store.Store) *ModerationService { func NewModerationService(st store.Store, perms *PermissionService) *ModerationService {
return &ModerationService{st: st} return &ModerationService{st: st, perms: perms}
}
// requireBanAuthority verifies the actor is allowed to ban/unban the target.
// The actor must hold BAN_MEMBERS (or Administrator, which bypasses permission
// checks) and must outrank the target in the role hierarchy — mirroring the
// position-based hierarchy used elsewhere (see admin/middleware.go and
// permissions.OwnerRolePosition). Returns ErrForbidden when either check fails.
func (s *ModerationService) requireBanAuthority(actorID, targetID int64) error {
if s.perms == nil {
// No permission service wired — fail closed rather than allow unchecked bans.
return fmt.Errorf("%w: permission service unavailable", ErrForbidden)
}
actorRole, err := s.perms.GetRoleForUser(actorID)
if err != nil || actorRole == nil {
return fmt.Errorf("%w: failed to load actor role", ErrForbidden)
}
if !permissions.HasAdmin(actorRole.Permissions) &&
!permissions.HasPerm(actorRole.Permissions, permissions.BanMembers) {
return fmt.Errorf("%w: missing BAN_MEMBERS permission", ErrForbidden)
}
// Role hierarchy: the actor must strictly outrank the target so a user
// cannot ban a peer or a higher-ranked user (e.g. the owner).
targetRole, err := s.perms.GetRoleForUser(targetID)
if err != nil || targetRole == nil {
return fmt.Errorf("%w: failed to load target role", ErrForbidden)
}
if actorRole.Position <= targetRole.Position {
return fmt.Errorf("%w: cannot moderate a user of equal or higher rank", ErrForbidden)
}
return nil
} }
// BanUser bans a target user. Validates the target exists and // BanUser bans a target user. Validates the target exists and
@@ -46,6 +82,11 @@ func (s *ModerationService) BanUser(actorID, targetID int64, reason string, expi
return fmt.Errorf("%w: user not found", ErrNotFound) return fmt.Errorf("%w: user not found", ErrNotFound)
} }
// Authorization: actor must hold BAN_MEMBERS and outrank the target.
if err := s.requireBanAuthority(actorID, targetID); err != nil {
return err
}
if err := s.st.BanUser(targetID, reason, expires); err != nil { if err := s.st.BanUser(targetID, reason, expires); err != nil {
return fmt.Errorf("%w: failed to ban user", ErrInternal) return fmt.Errorf("%w: failed to ban user", ErrInternal)
} }
@@ -69,6 +110,11 @@ func (s *ModerationService) UnbanUser(actorID, targetID int64) error {
return fmt.Errorf("%w: user not found", ErrNotFound) return fmt.Errorf("%w: user not found", ErrNotFound)
} }
// Authorization: actor must hold BAN_MEMBERS and outrank the target.
if err := s.requireBanAuthority(actorID, targetID); err != nil {
return err
}
if err := s.st.UnbanUser(targetID); err != nil { if err := s.st.UnbanUser(targetID); err != nil {
return fmt.Errorf("%w: failed to unban user", ErrInternal) return fmt.Errorf("%w: failed to unban user", ErrInternal)
} }
+1 -1
View File
@@ -36,7 +36,7 @@ func New(st store.Store, limiter *auth.RateLimiter) *Services {
DMs: NewDMService(st), DMs: NewDMService(st),
Invites: NewInviteService(st), Invites: NewInviteService(st),
Blocks: NewBlockService(st), Blocks: NewBlockService(st),
Moderation: NewModerationService(st), Moderation: NewModerationService(st, permSvc),
Voice: NewVoiceService(st, permSvc), Voice: NewVoiceService(st, permSvc),
} }
} }
+5
View File
@@ -60,6 +60,11 @@ func (s *UserService) ChangePassword(userID int64, newPasswordHash string, keepS
revoked, err := s.st.DeleteOtherSessions(userID, keepSessionID) revoked, err := s.st.DeleteOtherSessions(userID, keepSessionID)
if err != nil { if err != nil {
slog.Error("UserService.ChangePassword DeleteOtherSessions", "err", err, "user_id", userID) slog.Error("UserService.ChangePassword DeleteOtherSessions", "err", err, "user_id", userID)
// The password was updated, but other sessions could not be revoked, so
// devices authenticated under the old password remain valid. Surface this
// as a failure instead of silently reporting success — a password change
// is a security action and the caller must be able to warn/retry.
return revoked, fmt.Errorf("%w: password changed but failed to revoke other sessions", ErrInternal)
} }
_ = s.st.LogAudit(userID, "password_change", "user", userID, "password changed") _ = s.st.LogAudit(userID, "password_change", "user", userID, "password changed")
slog.Info("password changed", "user_id", userID, "sessions_revoked", revoked) slog.Info("password changed", "user_id", userID, "sessions_revoked", revoked)
+8 -4
View File
@@ -128,9 +128,9 @@ func (m *MemStore) SeedBlock(blockerID, blockedID int64) {
// ---------- Store interface: top-level ---------- // ---------- Store interface: top-level ----------
func (m *MemStore) Close() error { return nil } func (m *MemStore) Close() error { return nil }
func (m *MemStore) SQLDb() *sql.DB { return nil } func (m *MemStore) SQLDb() *sql.DB { return nil }
func (m *MemStore) WithTx(_ context.Context, fn func(Store) error) error { return fn(m) } func (m *MemStore) WithTx(_ context.Context, fn func(Store) error) error { return fn(m) }
// ---------- MessageStore ---------- // ---------- MessageStore ----------
@@ -686,7 +686,11 @@ func (m *MemStore) CreateAttachment(_ string, _ int64, _, _, _ string, _ int64,
} }
func (m *MemStore) GetAttachmentByID(_ string) (*db.Attachment, error) { func (m *MemStore) GetAttachmentByID(_ string) (*db.Attachment, error) {
panic("memstore: not implemented: GetAttachmentByID") // MemStore does not track attachments (CreateAttachment is unsupported and
// LinkAttachmentsToMessage is a no-op), so every lookup is "not found".
// Returning (nil, nil) rather than panicking keeps the attachment-ownership
// check in MessageService.SendMessage consistent with the no-op link path.
return nil, nil
} }
func (m *MemStore) GetAttachmentWithChannel(_ string) (*db.AttachmentAccess, error) { func (m *MemStore) GetAttachmentWithChannel(_ string) (*db.AttachmentAccess, error) {
+39
View File
@@ -117,11 +117,20 @@ type Updater struct {
cacheExpiry time.Time cacheExpiry time.Time
cachedErr error cachedErr error
errCacheExpiry time.Time errCacheExpiry time.Time
textAssetCache map[string]textAssetCacheEntry
mu syncutil.Mutex mu syncutil.Mutex
httpClient *http.Client httpClient *http.Client
signingKeyText string signingKeyText string
} }
// textAssetCacheEntry caches a small text asset (e.g. a client update .sig
// file) alongside the release cache so repeated requests are served from
// memory instead of re-fetching from GitHub on every call.
type textAssetCacheEntry struct {
content string
expiry time.Time
}
// NewUpdater creates an Updater for the given repository. // NewUpdater creates an Updater for the given repository.
func NewUpdater(currentVersion, githubToken, repoOwner, repoName string) *Updater { func NewUpdater(currentVersion, githubToken, repoOwner, repoName string) *Updater {
return &Updater{ return &Updater{
@@ -713,6 +722,36 @@ func (u *Updater) FetchTextAsset(ctx context.Context, url string) (string, error
return string(data), nil return string(data), nil
} }
// FetchTextAssetCached is FetchTextAsset with an in-memory cache keyed by URL,
// using the same cacheTTL as the release cache. It lets unauthenticated,
// unrate-limited callers (e.g. the client-update endpoint) be served from
// memory instead of triggering an outbound fetch on every request.
func (u *Updater) FetchTextAssetCached(ctx context.Context, url string) (string, error) {
now := time.Now()
u.mu.Lock()
if entry, ok := u.textAssetCache[url]; ok && now.Before(entry.expiry) {
content := entry.content
u.mu.Unlock()
return content, nil
}
u.mu.Unlock()
content, err := u.FetchTextAsset(ctx, url)
if err != nil {
return "", err
}
u.mu.Lock()
if u.textAssetCache == nil {
u.textAssetCache = make(map[string]textAssetCacheEntry)
}
u.textAssetCache[url] = textAssetCacheEntry{content: content, expiry: now.Add(cacheTTL)}
u.mu.Unlock()
return content, nil
}
// downloadFile downloads the content at url and writes it to destPath. // downloadFile downloads the content at url and writes it to destPath.
func (u *Updater) downloadFile(ctx context.Context, url, destPath string) error { func (u *Updater) downloadFile(ctx context.Context, url, destPath string) error {
req, err := http.NewRequestWithContext(ctx, http.MethodGet, url, nil) req, err := http.NewRequestWithContext(ctx, http.MethodGet, url, nil)
+1
View File
@@ -47,6 +47,7 @@ CREATE TABLE IF NOT EXISTS audit_log (
CREATE TABLE IF NOT EXISTS attachments ( CREATE TABLE IF NOT EXISTS attachments (
id TEXT PRIMARY KEY, id TEXT PRIMARY KEY,
message_id INTEGER REFERENCES messages(id) ON DELETE CASCADE, message_id INTEGER REFERENCES messages(id) ON DELETE CASCADE,
uploader_id INTEGER REFERENCES users(id),
filename TEXT NOT NULL, filename TEXT NOT NULL,
stored_as TEXT NOT NULL, stored_as TEXT NOT NULL,
mime_type TEXT NOT NULL, mime_type TEXT NOT NULL,
+39 -3
View File
@@ -83,9 +83,12 @@ func handlePluginCommand(ctx context.Context, h *Hub, c *Client, reqID string, p
} }
if result.Broadcast != "" && p.ChannelID != 0 { if result.Broadcast != "" && p.ChannelID != 0 {
// Verify the invoking client has permission to send to this channel // Verify the invoking client can post to this channel before broadcasting
// before broadcasting the plugin result to all channel members. // the plugin result to all channel members. Mirrors the normal send path:
if !h.requireChannelPerm(c, p.ChannelID, permissions.SendMessages, "SEND_MESSAGES") { // non-DM channels require READ_MESSAGES|SEND_MESSAGES (so a user cannot
// post into a channel they cannot read), and DM channels are validated by
// participant membership rather than role permissions.
if !h.requireChannelBroadcastAccess(c, p.ChannelID) {
return return
} }
// Channel broadcast — visible to everyone in the channel. // Channel broadcast — visible to everyone in the channel.
@@ -95,6 +98,39 @@ func handlePluginCommand(ctx context.Context, h *Hub, c *Client, reqID string, p
} }
} }
// requireChannelBroadcastAccess reports whether the client may post to
// channelID, mirroring the normal message-send permission path. DM channels are
// validated by participant membership; all other channels require
// READ_MESSAGES|SEND_MESSAGES. On failure it sends an error to the client and
// returns false. Routes through the shared permissions.Checker.RequireChannelAccess
// so DM handling matches the rest of the codebase.
func (h *Hub) requireChannelBroadcastAccess(c *Client, channelID int64) bool {
if c.user == nil {
c.sendMsg(buildErrorMsg(ErrCodeForbidden, "not authenticated"))
return false
}
ch, err := h.db.GetChannel(channelID)
if err != nil || ch == nil {
c.sendMsg(buildErrorMsg(ErrCodeNotFound, "channel not found"))
return false
}
role, err := h.db.GetRoleByID(c.user.RoleID)
if err != nil || role == nil {
c.sendMsg(buildErrorMsg(ErrCodeForbidden, "role not found"))
return false
}
if accessErr := h.permChecker.RequireChannelAccess(
c.userID, role.Permissions, role.ID, ch.Type, channelID,
permissions.ReadMessages|permissions.SendMessages,
); accessErr != nil {
slog.Warn("ws plugin broadcast permission denied",
"user_id", c.userID, "channel_id", channelID, "err", accessErr)
c.sendMsg(buildErrorMsg(ErrCodeForbidden, "missing permission to post in this channel"))
return false
}
return true
}
// buildCommandReply builds an ephemeral command_reply envelope. // buildCommandReply builds an ephemeral command_reply envelope.
func buildCommandReply(reqID, text string) []byte { func buildCommandReply(reqID, text string) []byte {
type payload struct { type payload struct {
+1
View File
@@ -43,6 +43,7 @@ CREATE TABLE IF NOT EXISTS audit_log (
CREATE TABLE IF NOT EXISTS attachments ( CREATE TABLE IF NOT EXISTS attachments (
id TEXT PRIMARY KEY, id TEXT PRIMARY KEY,
message_id INTEGER REFERENCES messages(id) ON DELETE CASCADE, message_id INTEGER REFERENCES messages(id) ON DELETE CASCADE,
uploader_id INTEGER REFERENCES users(id),
filename TEXT NOT NULL, filename TEXT NOT NULL,
stored_as TEXT NOT NULL, stored_as TEXT NOT NULL,
mime_type TEXT NOT NULL, mime_type TEXT NOT NULL,
+12
View File
@@ -3,6 +3,7 @@ package ws
import ( import (
"context" "context"
"encoding/json" "encoding/json"
"fmt"
) )
// registerVoiceHandlersV1 registers voice handlers that remain V1 (complex // registerVoiceHandlersV1 registers voice handlers that remain V1 (complex
@@ -12,6 +13,17 @@ func registerVoiceHandlersV1(r *HandlerRegistry) {
h.handleVoiceJoin(ctx, c, payload) h.handleVoiceJoin(ctx, c, payload)
}) })
r.Register(MsgTypeVoiceLeave, func(ctx context.Context, h *Hub, c *Client, _ string, _ json.RawMessage) { r.Register(MsgTypeVoiceLeave, func(ctx context.Context, h *Hub, c *Client, _ string, _ json.RawMessage) {
// Rate limit only the explicit client-initiated voice_leave message.
// handleVoiceLeave is also invoked internally for disconnect and
// channel-switch cleanup (serve.go, voice_join.go); those paths must
// never be throttled or they would leak ghost voice states, so the
// limit lives here in the dispatch wrapper rather than inside the shared
// handleVoiceLeave routine. Mirrors the voice control Limiter idiom.
ratKey := fmt.Sprintf("voice_leave:%d", c.userID)
if h.limiter != nil && !h.limiter.Allow(ratKey, voiceLeaveRateLimit, voiceLeaveWindow) {
c.sendMsg(buildErrorMsg(ErrCodeRateLimited, "too many voice leave attempts"))
return
}
h.handleVoiceLeave(ctx, c) h.handleVoiceLeave(ctx, c)
}) })
} }
+23 -41
View File
@@ -2,9 +2,7 @@ package ws
import ( import (
"context" "context"
"encoding/json"
"fmt" "fmt"
"io"
"log/slog" "log/slog"
"net/http" "net/http"
"strconv" "strconv"
@@ -12,8 +10,13 @@ import (
"github.com/livekit/protocol/auth" "github.com/livekit/protocol/auth"
"github.com/livekit/protocol/livekit" "github.com/livekit/protocol/livekit"
"github.com/livekit/protocol/webhook"
) )
// webhookMaxBodyBytes bounds the webhook request body to prevent unbounded
// reads from an unauthenticated caller.
const webhookMaxBodyBytes = 64 * 1024
// NewLiveKitWebhookHandler returns an HTTP handler that processes LiveKit // NewLiveKitWebhookHandler returns an HTTP handler that processes LiveKit
// webhook events. It synchronises LiveKit room state back into OwnCord's // webhook events. It synchronises LiveKit room state back into OwnCord's
// voice_states DB — primarily for crash recovery when a participant // voice_states DB — primarily for crash recovery when a participant
@@ -22,56 +25,35 @@ import (
// Speaker detection is handled client-side via LiveKit's // Speaker detection is handled client-side via LiveKit's
// RoomEvent.ActiveSpeakersChanged (lower latency than webhooks). // RoomEvent.ActiveSpeakersChanged (lower latency than webhooks).
func (h *Hub) NewLiveKitWebhookHandler(apiKey, apiSecret string) http.HandlerFunc { func (h *Hub) NewLiveKitWebhookHandler(apiKey, apiSecret string) http.HandlerFunc {
// The SDK receiver verifies the token signature AND that the token's sha256
// claim matches the request body hash, binding verification to the body so
// a captured token cannot be replayed against a forged payload.
provider := auth.NewSimpleKeyProvider(apiKey, apiSecret)
return func(w http.ResponseWriter, r *http.Request) { return func(w http.ResponseWriter, r *http.Request) {
// Check Authorization header BEFORE reading the body to avoid // Check Authorization header BEFORE reading the body to avoid
// allocating memory for unauthenticated requests. // allocating memory for unauthenticated requests.
authHeader := r.Header.Get("Authorization") if r.Header.Get("Authorization") == "" {
if authHeader == "" {
slog.Warn("livekit webhook: missing Authorization header") slog.Warn("livekit webhook: missing Authorization header")
http.Error(w, "unauthorized", http.StatusUnauthorized) http.Error(w, "unauthorized", http.StatusUnauthorized)
return return
} }
body, err := io.ReadAll(io.LimitReader(r.Body, 64*1024)) // Bound the body before the SDK reads it (ReceiveWebhookEvent uses an
// unbounded io.ReadAll internally).
r.Body = http.MaxBytesReader(w, r.Body, webhookMaxBodyBytes)
// ReceiveWebhookEvent verifies the JWT signature, the token's body-hash
// claim, and the exp/nbf claims, then parses the payload. This replaces
// the previous manual ParseAPIToken/Verify sequence, which was not bound
// to the request body (forgery/replay).
event, err := webhook.ReceiveWebhookEvent(r, provider)
if err != nil { if err != nil {
slog.Error("livekit webhook: read body failed", "error", err) slog.Warn("livekit webhook: verification failed", "error", err)
http.Error(w, "bad request", http.StatusBadRequest)
return
}
// LiveKit sends "Bearer <token>" in the Authorization header.
tokenStr := strings.TrimPrefix(authHeader, "Bearer ")
verifier, err := auth.ParseAPIToken(tokenStr)
if err != nil {
slog.Warn("livekit webhook: invalid token", "error", err)
http.Error(w, "unauthorized", http.StatusUnauthorized) http.Error(w, "unauthorized", http.StatusUnauthorized)
return return
} }
if verifier.APIKey() != apiKey {
slog.Warn("livekit webhook: API key mismatch",
"got", verifier.APIKey(), "want", apiKey)
http.Error(w, "unauthorized", http.StatusUnauthorized)
return
}
// Verify checks both the HMAC signature and the exp/nbf claims
// (via jwt.Claims.Validate with Time: time.Now() inside the SDK).
// Expired tokens are rejected with an error here.
if _, _, err := verifier.Verify(apiSecret); err != nil {
slog.Warn("livekit webhook: token verification failed", "error", err)
http.Error(w, "unauthorized", http.StatusUnauthorized)
return
}
// Parse the webhook event payload.
var event livekit.WebhookEvent
if err := json.Unmarshal(body, &event); err != nil {
slog.Warn("livekit webhook: invalid JSON", "error", err)
http.Error(w, "bad request", http.StatusBadRequest)
return
}
slog.Info("livekit webhook received", slog.Info("livekit webhook received",
"event", event.Event, "event", event.Event,
"room", event.GetRoom().GetName(), "room", event.GetRoom().GetName(),
@@ -80,9 +62,9 @@ func (h *Hub) NewLiveKitWebhookHandler(apiKey, apiSecret string) http.HandlerFun
switch event.Event { switch event.Event {
case "participant_joined": case "participant_joined":
h.handleWebhookParticipantJoined(r.Context(), &event) h.handleWebhookParticipantJoined(r.Context(), event)
case "participant_left": case "participant_left":
h.handleWebhookParticipantLeft(r.Context(), &event) h.handleWebhookParticipantLeft(r.Context(), event)
default: default:
slog.Debug("livekit webhook: unhandled event", "event", event.Event) slog.Debug("livekit webhook: unhandled event", "event", event.Event)
} }
+22 -1
View File
@@ -3,6 +3,17 @@ package ws
import ( import (
"context" "context"
"encoding/base64" "encoding/base64"
"fmt"
"time"
)
// Voice E2EE rate limits. Both the announce and offer relays fan out to every
// other voice participant (and an offer can force a key rotation / disconnect
// for peers), so a single user must not be able to spam them. Mirrors the
// named-constant Limiter idiom used by the voice control handlers.
const (
voiceE2EERateLimit = 5
voiceE2EEWindow = time.Second
) )
// validateBase64Loose checks that s is valid padded (StdEncoding) or unpadded // validateBase64Loose checks that s is valid padded (StdEncoding) or unpadded
@@ -82,11 +93,16 @@ func (h *Hub) computeIsKeyHolder(channelID, userID int64) bool {
// It validates the public key and returns a SetE2EEPubKey mutation plus a // It validates the public key and returns a SetE2EEPubKey mutation plus a
// VoiceE2EEAnnounceEvent for relay to other voice channel participants. // VoiceE2EEAnnounceEvent for relay to other voice channel participants.
func handleVoiceE2EEAnnounceV2(_ context.Context, cmd Command, info ClientInfo, deps any) Result { func handleVoiceE2EEAnnounceV2(_ context.Context, cmd Command, info ClientInfo, deps any) Result {
_ = deps.(VoiceDeps) d := deps.(VoiceDeps)
announceCmd := cmd.(VoiceE2EEAnnounceCmd) announceCmd := cmd.(VoiceE2EEAnnounceCmd)
userID := info.UserID userID := info.UserID
voiceChID := info.VoiceChannelID voiceChID := info.VoiceChannelID
ratKey := fmt.Sprintf("voice_e2ee_announce:%d", userID)
if d.Limiter != nil && !d.Limiter.Allow(ratKey, voiceE2EERateLimit, voiceE2EEWindow) {
return Result{Error: ClientError{Code: ErrCodeRateLimited, Message: "too many e2ee announcements"}}
}
if voiceChID == 0 { if voiceChID == 0 {
return Result{Error: ClientError{Code: ErrCodeVoiceError, Message: "not in a voice channel"}} return Result{Error: ClientError{Code: ErrCodeVoiceError, Message: "not in a voice channel"}}
} }
@@ -131,6 +147,11 @@ func handleVoiceE2EEOfferV2(_ context.Context, cmd Command, info ClientInfo, dep
offerCmd := cmd.(VoiceE2EEOfferCmd) offerCmd := cmd.(VoiceE2EEOfferCmd)
voiceChID := info.VoiceChannelID voiceChID := info.VoiceChannelID
ratKey := fmt.Sprintf("voice_e2ee_offer:%d", info.UserID)
if d.Limiter != nil && !d.Limiter.Allow(ratKey, voiceE2EERateLimit, voiceE2EEWindow) {
return Result{Error: ClientError{Code: ErrCodeRateLimited, Message: "too many e2ee offers"}}
}
if voiceChID == 0 { if voiceChID == 0 {
return Result{Error: ClientError{Code: ErrCodeVoiceError, Message: "not in a voice channel"}} return Result{Error: ClientError{Code: ErrCodeVoiceError, Message: "not in a voice channel"}}
} }
+22
View File
@@ -12,6 +12,19 @@ import (
"github.com/owncord/server/permissions" "github.com/owncord/server/permissions"
) )
// Voice join/leave rate limits. voice_join and voice_leave each fan out a
// broadcast to every connected client, so a single user must not be able to
// trigger them in a tight loop. Mirrors the named-constant idiom used by the
// voice control handlers (see voice_broadcast.go / voice_controls.go).
// voiceLeaveRateLimit/Window are consumed by the voice_leave message dispatch
// in handlers_voice.go (same package).
const (
voiceJoinRateLimit = 5
voiceJoinWindow = time.Second
voiceLeaveRateLimit = 5
voiceLeaveWindow = time.Second
)
// validVoiceQuality returns true if q is an accepted voice quality preset. // validVoiceQuality returns true if q is an accepted voice quality preset.
// Uses voiceQualities (defined in voice_broadcast.go) as the single source of truth. // Uses voiceQualities (defined in voice_broadcast.go) as the single source of truth.
func validVoiceQuality(q string) bool { func validVoiceQuality(q string) bool {
@@ -30,6 +43,15 @@ func validVoiceQuality(q string) bool {
// 8. Broadcasts voice_state to all clients. // 8. Broadcasts voice_state to all clients.
// 9. Sends voice_config to the joiner. // 9. Sends voice_config to the joiner.
func (h *Hub) handleVoiceJoin(ctx context.Context, c *Client, payload json.RawMessage) { func (h *Hub) handleVoiceJoin(ctx context.Context, c *Client, payload json.RawMessage) {
// Rate limit: voice_join broadcasts a voice_state update to every connected
// client, so cap how often a single user can trigger the fan-out. Mirrors the
// Limiter.Allow(...) idiom used by the voice control handlers.
ratKey := fmt.Sprintf("voice_join:%d", c.userID)
if h.limiter != nil && !h.limiter.Allow(ratKey, voiceJoinRateLimit, voiceJoinWindow) {
c.sendMsg(buildErrorMsg(ErrCodeRateLimited, "too many voice join attempts"))
return
}
channelID, err := parseChannelID(payload) channelID, err := parseChannelID(payload)
if err != nil || channelID <= 0 { if err != nil || channelID <= 0 {
c.sendMsg(buildErrorMsg(ErrCodeBadRequest, "channel_id must be a positive integer")) c.sendMsg(buildErrorMsg(ErrCodeBadRequest, "channel_id must be a positive integer"))