From c148cb8ce7618f33246cfa272fb02be6e4297de9 Mon Sep 17 00:00:00 2001 From: Deivid Soto Date: Fri, 15 May 2026 17:10:42 +0200 Subject: [PATCH 01/11] fix(security): harden HLS session IDs, /health disclosure, archive password handling Phase 1 security audit follow-up: - Reject HLS session IDs that aren't safe filesystem components (regex allowlist) to defend against path traversal via a buggy or compromised server. Applied at StartHLSSession and at the /hls URL handler; invalid IDs share the 404 of unknown sessions so the accepted format isn't enumerable. - /health no longer leaks the active filename, taskID prefix or client IP to non-loopback callers. Uses net.IP.IsLoopback so IPv4-mapped IPv6 (::ffff:127.0.0.1) is recognised and the empty-string parse failure stops bypassing the boundary. - unrar/7z passwords now travel through stdin instead of -p in argv, removing /proc//cmdline disclosure. Control characters in the password are rejected up front so a hostile NZB cannot feed extra prompt answers. Both invocations are bounded by a 30-minute context to stop indefinite hangs if the tool ever decides to prompt. --- internal/engine/hls.go | 3 + internal/engine/hls_test.go | 30 ++++++++++ internal/engine/stream_server.go | 33 +++++++++-- internal/engine/stream_server_test.go | 81 ++++++++++++++++++++++++++ internal/engine/validate.go | 12 ++++ internal/usenet/postprocess/extract.go | 70 ++++++++++++++++++---- 6 files changed, 213 insertions(+), 16 deletions(-) create mode 100644 internal/engine/validate.go diff --git a/internal/engine/hls.go b/internal/engine/hls.go index 537a79b..03a9948 100644 --- a/internal/engine/hls.go +++ b/internal/engine/hls.go @@ -241,6 +241,9 @@ func StartHLSSession(ctx context.Context, cfg HLSSessionConfig) (*HLSSession, er if cfg.SessionID == "" { return nil, errors.New("hls: empty session id") } + if !validSessionID.MatchString(cfg.SessionID) { + return nil, errors.New("hls: invalid session id") + } if cfg.SourcePath == "" { return nil, errors.New("hls: empty source path") } diff --git a/internal/engine/hls_test.go b/internal/engine/hls_test.go index 0aea35d..7c7cfa4 100644 --- a/internal/engine/hls_test.go +++ b/internal/engine/hls_test.go @@ -261,3 +261,33 @@ func TestCleanupHLSOrphanDirsMissingRoot(t *testing.T) { t.Errorf("CleanupHLSOrphanDirs on missing root = %v, want nil", err) } } + +func TestValidSessionID(t *testing.T) { + good := []string{ + "abc", + "7b8c4f12-9d3e-4a1b-9c2f-aabbccddeeff", + "ABC_123-xyz", + strings.Repeat("a", 128), + } + bad := []string{ + "", + "../etc/passwd", + "foo/bar", + "foo\\bar", + "foo.bar", + "with spaces", + "with\nnewline", + strings.Repeat("a", 129), + "héctor", // non-ascii + } + for _, id := range good { + if !validSessionID.MatchString(id) { + t.Errorf("validSessionID rejected good id %q", id) + } + } + for _, id := range bad { + if validSessionID.MatchString(id) { + t.Errorf("validSessionID accepted bad id %q", id) + } + } +} diff --git a/internal/engine/stream_server.go b/internal/engine/stream_server.go index 2e42868..7440979 100644 --- a/internal/engine/stream_server.go +++ b/internal/engine/stream_server.go @@ -303,6 +303,12 @@ func (ss *StreamServer) hlsHandler(w http.ResponseWriter, r *http.Request) { return } sessionID := parts[0] + // Reject malformed IDs with the same 404 we return for unknown sessions — + // no oracle for the accepted format. + if !validSessionID.MatchString(sessionID) { + http.Error(w, "hls session not found", http.StatusNotFound) + return + } session := ss.hls.Get(sessionID) if session == nil { http.Error(w, "hls session not found", http.StatusNotFound) @@ -392,6 +398,17 @@ func (ss *StreamServer) healthHandler(w http.ResponseWriter, r *http.Request) { ss.mu.RUnlock() clientIP, _, _ := net.SplitHostPort(r.RemoteAddr) + // Only expose filename/taskID/client to loopback callers (local diagnostics). + // Remote callers (LAN, Tailscale, UPnP public) get a minimal probe response + // so that scanners and unauthenticated peers cannot fingerprint the active + // download. The web stream-probe only checks HTTP 200 + Content-Type. + // + // Use net.IP.IsLoopback so we also accept ::ffff:127.0.0.1 (Linux dual-stack + // IPv4-mapped form) and reject the empty-string fallthrough when + // SplitHostPort fails on a malformed RemoteAddr — both would otherwise + // silently bypass the disclosure boundary. + parsedIP := net.ParseIP(clientIP) + isLocal := parsedIP != nil && parsedIP.IsLoopback() type healthResponse struct { Status string `json:"status"` @@ -399,19 +416,23 @@ func (ss *StreamServer) healthHandler(w http.ResponseWriter, r *http.Request) { File string `json:"file,omitempty"` Task string `json:"task,omitempty"` Port int `json:"port"` - Client string `json:"client"` + Client string `json:"client,omitempty"` } resp := healthResponse{ Status: "ok", Port: ss.port, - Client: clientIP, } if provider != nil { resp.Streaming = true - resp.File = provider.FileName() - resp.Task = taskID - if len(resp.Task) > 8 { - resp.Task = resp.Task[:8] + } + if isLocal { + resp.Client = clientIP + if provider != nil { + resp.File = provider.FileName() + resp.Task = taskID + if len(resp.Task) > 8 { + resp.Task = resp.Task[:8] + } } } diff --git a/internal/engine/stream_server_test.go b/internal/engine/stream_server_test.go index 623a16d..3c58b1a 100644 --- a/internal/engine/stream_server_test.go +++ b/internal/engine/stream_server_test.go @@ -5,6 +5,7 @@ import ( "fmt" "io" "net/http" + "net/http/httptest" "strings" "sync" "testing" @@ -379,6 +380,86 @@ func TestStreamServer_Health_WithFile(t *testing.T) { } } +// TestStreamServer_Health_NonLoopback_NoLeak verifica que /health no revela +// nombre de fichero, taskID ni client IP cuando el caller no es loopback. +// Protección contra reconnaissance vía LAN / UPnP / Tailscale. +func TestStreamServer_Health_NonLoopback_NoLeak(t *testing.T) { + srv := NewStreamServer(0) + srv.disableUPnP = true + ctx := context.Background() + if err := srv.Listen(ctx); err != nil { + t.Fatalf("Listen() error: %v", err) + } + defer srv.Shutdown(ctx) + + provider := newFakeProvider("secret.mkv", []byte("data")) + srv.SetFile(provider, "secret-task-id") + + cases := []struct { + name string + remoteAddr string + }{ + {"lan_ipv4", "192.168.1.50:54321"}, + {"empty_host_no_bypass", ":54321"}, + {"public_ipv4", "203.0.113.10:443"}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + rr := httptest.NewRecorder() + req := httptest.NewRequest(http.MethodGet, "/health", nil) + req.RemoteAddr = tc.remoteAddr + srv.healthHandler(rr, req) + + body := rr.Body.String() + if !strings.Contains(body, `"status":"ok"`) { + t.Errorf("body missing status:ok: %q", body) + } + if !strings.Contains(body, `"streaming":true`) { + t.Errorf("body should report streaming bool: %q", body) + } + if strings.Contains(body, "secret.mkv") { + t.Errorf("body leaked filename: %q", body) + } + if strings.Contains(body, "secret-t") { + t.Errorf("body leaked task id: %q", body) + } + if strings.Contains(body, "192.168.1.50") || strings.Contains(body, "203.0.113.10") { + t.Errorf("body leaked client ip: %q", body) + } + }) + } +} + +// TestStreamServer_HLS_InvalidSessionID verifica que el hlsHandler rechaza +// session IDs con caracteres ilegales devolviendo 404 (uniforme con sesión +// inexistente) para no filtrar el formato aceptado a un attacker. +func TestStreamServer_HLS_InvalidSessionID(t *testing.T) { + srv := NewStreamServer(0) + srv.disableUPnP = true + ctx := context.Background() + if err := srv.Listen(ctx); err != nil { + t.Fatalf("Listen() error: %v", err) + } + defer srv.Shutdown(ctx) + + bad := []string{ + "/hls/..%2Fetc%2Fpasswd/master.m3u8", + "/hls/foo.bar/master.m3u8", + "/hls/foo%20bar/master.m3u8", + "/hls/foo%2Fbar/master.m3u8", + } + for _, path := range bad { + t.Run(path, func(t *testing.T) { + rr := httptest.NewRecorder() + req := httptest.NewRequest(http.MethodGet, path, nil) + srv.hlsHandler(rr, req) + if rr.Code != http.StatusNotFound { + t.Errorf("path %q: status = %d, want 404", path, rr.Code) + } + }) + } +} + // TestStreamServer_MKV_ContentType verifica que el Content-Type para .mkv // es el correcto. func TestStreamServer_MKV_ContentType(t *testing.T) { diff --git a/internal/engine/validate.go b/internal/engine/validate.go new file mode 100644 index 0000000..288a41b --- /dev/null +++ b/internal/engine/validate.go @@ -0,0 +1,12 @@ +// Package engine — validate.go centralises input validators used by the +// stream/HLS HTTP handlers and the daemon glue. Keep new validators in this +// file so a future reviewer can audit the trust boundary in one place. +package engine + +import "regexp" + +// validSessionID restricts session IDs to characters safe for use as a single +// filesystem path component. Server-issued UUIDs and hex strings match this; +// anything containing slashes, dots, or path separators is rejected so a +// compromised or buggy server cannot escape hlsTmpDirRoot via os.MkdirAll. +var validSessionID = regexp.MustCompile(`^[a-zA-Z0-9_-]{1,128}$`) diff --git a/internal/usenet/postprocess/extract.go b/internal/usenet/postprocess/extract.go index 0a9b582..0c6a8e3 100644 --- a/internal/usenet/postprocess/extract.go +++ b/internal/usenet/postprocess/extract.go @@ -1,6 +1,7 @@ package postprocess import ( + "context" "fmt" "log" "os" @@ -8,8 +9,25 @@ import ( "path/filepath" "regexp" "strings" + "time" ) +// extractTimeout caps how long a single extractor invocation may run. Without +// a cap, an encrypted archive that triggers a TTY-only prompt (or a corrupt +// archive that confuses the tool) hangs the post-process pipeline forever. +const extractTimeout = 30 * time.Minute + +// validatePassword rejects passwords containing control characters that could +// inject extra answers into unrar/7z prompts via stdin (e.g. a newline lets an +// attacker-controlled NZB password feed a second response to overwrite or +// rename prompts). +func validatePassword(password string) error { + if strings.ContainsAny(password, "\r\n\x00") { + return fmt.Errorf("invalid password: contains control characters") + } + return nil +} + // ExtractorType identifies which extraction tool is available. type ExtractorType string @@ -50,18 +68,35 @@ func Extract(archivePath string, outputDir string, password string) ([]string, e } // extractUnrar extracts using unrar. +// +// Security: when a password is supplied it is sent via stdin rather than via +// the `-p` switch so it does not appear in `/proc//cmdline` +// (visible to any other process on the host). unrar prompts for the password +// when no `-p` switch is given, and reads the prompt response from stdin when +// no controlling TTY is attached (the usual case for a daemon-spawned child). func extractUnrar(unrarPath, archivePath, outputDir, password string) ([]string, error) { + if err := validatePassword(password); err != nil { + return nil, err + } args := []string{"x", "-o+", "-y"} - if password != "" { - args = append(args, "-p"+password) - } else { - args = append(args, "-p-") // no password, skip asking + if password == "" { + // Tell unrar there is no password so it skips the prompt and fails + // fast on encrypted archives instead of hanging. + args = append(args, "-p-") } args = append(args, archivePath, outputDir+"/") - cmd := exec.Command(unrarPath, args...) + ctx, cancel := context.WithTimeout(context.Background(), extractTimeout) + defer cancel() + cmd := exec.CommandContext(ctx, unrarPath, args...) cmd.Dir = outputDir + if password != "" { + cmd.Stdin = strings.NewReader(password + "\n") + } output, err := cmd.CombinedOutput() + if ctx.Err() == context.DeadlineExceeded { + return nil, fmt.Errorf("unrar: timed out after %s", extractTimeout) + } if err != nil { // Check for password error outStr := string(output) @@ -75,18 +110,33 @@ func extractUnrar(unrarPath, archivePath, outputDir, password string) ([]string, } // extract7z extracts using 7z. +// +// Security: same rationale as extractUnrar — passwords go through stdin to +// avoid `/proc//cmdline` exposure. 7z reads the password from stdin when +// no `-p` switch is given and the archive is encrypted. func extract7z(szPath, archivePath, outputDir, password string) ([]string, error) { + if err := validatePassword(password); err != nil { + return nil, err + } args := []string{"x", "-y", "-o" + outputDir} - if password != "" { - args = append(args, "-p"+password) - } else { - args = append(args, "-p") // empty password + if password == "" { + // `-p` with no value tells 7z the password is empty so encrypted + // archives fail fast instead of waiting for a prompt. + args = append(args, "-p") } args = append(args, archivePath) - cmd := exec.Command(szPath, args...) + ctx, cancel := context.WithTimeout(context.Background(), extractTimeout) + defer cancel() + cmd := exec.CommandContext(ctx, szPath, args...) cmd.Dir = outputDir + if password != "" { + cmd.Stdin = strings.NewReader(password + "\n") + } output, err := cmd.CombinedOutput() + if ctx.Err() == context.DeadlineExceeded { + return nil, fmt.Errorf("7z: timed out after %s", extractTimeout) + } if err != nil { outStr := string(output) if strings.Contains(outStr, "Wrong password") || strings.Contains(outStr, "incorrect password") { From 433e375defea7e3666c6cb5e837494fa6945134f Mon Sep 17 00:00:00 2001 From: Deivid Soto Date: Fri, 15 May 2026 17:29:22 +0200 Subject: [PATCH 02/11] fix(security): UPnP opt-in, bounded SSE reader, signed self-update MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Phase 2 security audit follow-up. Three independent hardenings against the unauthenticated daemon surface, the long-lived agent SSE stream and the self-update channel. UPnP is now opt-in. The stream port + /hls endpoints have no auth, so publishing them on the WAN via the gateway was a default that exposed active downloads to anyone scanning the operator's external IP. New config downloads.enable_upnp (default false) gates the mapping; LAN and Tailscale clients continue to work unchanged. A startup log makes the new default visible. The agent SSE reader now uses a bounded bufio.Scanner instead of an unbounded ReadString. A hostile or buggy server can no longer grow daemon memory by streaming a single line forever or by emitting unbounded data: continuation lines — both are capped at 256 KiB and 1 MiB respectively, and an error is surfaced so SignalLoop reconnects. Self-update now verifies an ed25519 signature over checksums.txt when the binary was built with a release public key embedded (injected via goreleaser ldflags from RELEASE_SIGNING_PUBKEY). The companion scripts/sign-checksums runs in the release workflow when both the public-key variable and the private-key secret are present, uploading checksums.txt.sig next to the existing checksums file. Builds without the embedded key continue to update with SHA256-only verification; a --allow-unsigned flag is provided so users on a signed build can still install pre-signing releases or recover from an accidental unsigned release. A new scripts/gen-release-key helper documents the one-time keypair generation procedure required before flipping signing on. --- .github/workflows/release.yml | 22 ++++ .goreleaser.yml | 4 + internal/agent/signal_client.go | 47 ++++++--- internal/agent/signal_client_test.go | 43 ++++++++ internal/cmd/daemon.go | 1 + internal/cmd/self_update.go | 10 +- internal/cmd/upgrade.go | 7 +- internal/config/config.go | 1 + internal/engine/stream_server.go | 28 +++++- internal/engine/stream_server_test.go | 6 +- internal/engine/watch_reporter_test.go | 3 +- internal/upgrade/download.go | 36 ++++++- internal/upgrade/signature.go | 112 +++++++++++++++++++++ internal/upgrade/signature_test.go | 134 +++++++++++++++++++++++++ internal/upgrade/upgrade.go | 32 +++++- scripts/gen-release-key/main.go | 37 +++++++ scripts/sign-checksums/main.go | 60 +++++++++++ 17 files changed, 551 insertions(+), 32 deletions(-) create mode 100644 internal/upgrade/signature.go create mode 100644 internal/upgrade/signature_test.go create mode 100644 scripts/gen-release-key/main.go create mode 100644 scripts/sign-checksums/main.go diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 8283150..ea07be7 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -27,6 +27,28 @@ jobs: env: GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} SENTRY_DSN: ${{ secrets.SENTRY_DSN }} + # Empty when RELEASE_SIGNING_PUBKEY variable is unset — goreleaser + # accepts it and the resulting binary disables signature checks + # (back-compat: pre-signing releases continue to update). Set + # RELEASE_SIGNING_PUBKEY (variable) + RELEASE_SIGNING_KEY (secret) + # to turn verification on. + RELEASE_SIGNING_PUBKEY: ${{ vars.RELEASE_SIGNING_PUBKEY }} + + - name: Sign checksums.txt with ed25519 + # Reference secrets.X directly — step-level env defined in this same + # step is unreliable to read from this step's own if: expression. + if: ${{ vars.RELEASE_SIGNING_PUBKEY != '' && secrets.RELEASE_SIGNING_KEY != '' }} + env: + RELEASE_SIGNING_KEY: ${{ secrets.RELEASE_SIGNING_KEY }} + RELEASE_TAG: ${{ github.ref_name }} + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + run: | + set -euo pipefail + go run ./scripts/sign-checksums \ + -key "$RELEASE_SIGNING_KEY" \ + -in dist/checksums.txt \ + -out dist/checksums.txt.sig + gh release upload "$RELEASE_TAG" dist/checksums.txt.sig --clobber docker: needs: release diff --git a/.goreleaser.yml b/.goreleaser.yml index 0a5c821..26ce802 100644 --- a/.goreleaser.yml +++ b/.goreleaser.yml @@ -26,6 +26,10 @@ builds: - -s -w - -X github.com/torrentclaw/unarr/internal/cmd.Version={{.Version}} - -X github.com/torrentclaw/unarr/internal/sentry.dsn={{ .Env.SENTRY_DSN }} + # Release-signing public key — verified by the self-updater against + # checksums.txt.sig. Empty when not configured; in that case + # signature verification is skipped and a warning is logged. + - -X github.com/torrentclaw/unarr/internal/upgrade.releasePubKeyBase64={{ .Env.RELEASE_SIGNING_PUBKEY }} archives: - formats: [tar.gz] diff --git a/internal/agent/signal_client.go b/internal/agent/signal_client.go index e41a9ea..624dc6c 100644 --- a/internal/agent/signal_client.go +++ b/internal/agent/signal_client.go @@ -140,26 +140,29 @@ func (c *Client) OpenSignalStream(ctx context.Context, sessionID string) (*Signa return stream, nil } +// sseMaxLineBytes caps the size of a single SSE line. Real signalling lines +// are JSON payloads of a few hundred bytes; 256 KiB is generous enough to +// survive a future schema bump but small enough that a hostile or buggy +// server cannot grow daemon memory by streaming a single line forever. +const sseMaxLineBytes = 256 * 1024 + +// sseMaxEventBytes caps the total bytes buffered across the lines of one +// SSE event. Without a cap, a peer could send unbounded `data:` continuation +// lines and OOM the daemon between blank-line dispatches. +const sseMaxEventBytes = 1024 * 1024 + func (s *SignalEventStream) read() { defer close(s.done) defer close(s.events) - reader := bufio.NewReaderSize(s.resp.Body, 16*1024) + scanner := bufio.NewScanner(s.resp.Body) + scanner.Buffer(make([]byte, 16*1024), sseMaxLineBytes) + var dataBuf bytes.Buffer var eventName string - for { - line, err := reader.ReadString('\n') - if err != nil { - if err != io.EOF { - select { - case s.errs <- err: - default: - } - } - return - } - line = strings.TrimRight(line, "\r\n") + for scanner.Scan() { + line := strings.TrimRight(scanner.Text(), "\r") if line == "" { // End of an event — dispatch if we have data. if dataBuf.Len() == 0 { @@ -190,6 +193,18 @@ func (s *SignalEventStream) read() { } if strings.HasPrefix(line, "data:") { payload := strings.TrimSpace(line[len("data:"):]) + // Refuse to grow the event buffer past the cap. Reset so a + // well-formed event after the offender can still be parsed, + // and surface an error so SignalLoop reconnects. + if dataBuf.Len()+len(payload)+1 > sseMaxEventBytes { + dataBuf.Reset() + eventName = "" + select { + case s.errs <- fmt.Errorf("sse: event exceeded %d bytes", sseMaxEventBytes): + default: + } + return + } if dataBuf.Len() > 0 { dataBuf.WriteByte('\n') } @@ -198,6 +213,12 @@ func (s *SignalEventStream) read() { } // id:, retry:, anything else — ignore for now. } + if err := scanner.Err(); err != nil { + select { + case s.errs <- err: + default: + } + } } // SignalLoop runs an SSE consumer that reconnects automatically on disconnect. diff --git a/internal/agent/signal_client_test.go b/internal/agent/signal_client_test.go index 2527890..796b545 100644 --- a/internal/agent/signal_client_test.go +++ b/internal/agent/signal_client_test.go @@ -6,6 +6,7 @@ import ( "fmt" "net/http" "net/http/httptest" + "strings" "sync" "testing" "time" @@ -120,6 +121,48 @@ func TestSignalStreamCloseCancelsRead(t *testing.T) { wg.Wait() } +// TestSignalStreamRejectsOversizedEvent verifies that a hostile or buggy +// server sending an unbounded `data:` event surfaces an error and stops +// the reader instead of growing daemon memory forever. +func TestSignalStreamRejectsOversizedEvent(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.Header.Get("Authorization") != "Bearer test-key" { + http.Error(w, "auth", http.StatusUnauthorized) + return + } + w.Header().Set("Content-Type", "text/event-stream") + flusher := w.(http.Flusher) + // Send many data: continuation lines until we blow past the + // per-event cap. Each chunk is a short legitimate-looking line. + chunk := "data: " + strings.Repeat("x", 4096) + "\n" + fmt.Fprint(w, "event: signal\n") + for i := 0; i < (sseMaxEventBytes/4096)+8; i++ { + fmt.Fprint(w, chunk) + } + flusher.Flush() + <-r.Context().Done() + })) + defer srv.Close() + + c := NewClient(srv.URL, "test-key", "test-ua") + ctx, cancel := context.WithTimeout(context.Background(), 3*time.Second) + defer cancel() + + stream, err := c.OpenSignalStream(ctx, "session-overflow") + if err != nil { + t.Fatalf("open: %v", err) + } + defer stream.Close() + + for range stream.Events() { + // Should never receive a parsed event — the over-sized buffer must + // be rejected before dispatch. + } + if err := stream.Err(); err == nil { + t.Fatal("expected error from oversized event, got nil") + } +} + func TestPostSignalSendsCorrectBody(t *testing.T) { var bodySeen map[string]any srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { diff --git a/internal/cmd/daemon.go b/internal/cmd/daemon.go index 84a1245..0964e0f 100644 --- a/internal/cmd/daemon.go +++ b/internal/cmd/daemon.go @@ -240,6 +240,7 @@ func runDaemonStart() error { // Create persistent stream server streamSrv := engine.NewStreamServer(cfg.Download.StreamPort) + streamSrv.SetUPnPEnabled(cfg.Download.EnableUPnP) // Reap HLS tmpdirs left over from a previous daemon run before we start // accepting new sessions. The in-memory registry doesn't survive a // restart, so without this disk usage grows unbounded across restarts. diff --git a/internal/cmd/self_update.go b/internal/cmd/self_update.go index 31fb891..e68a5de 100644 --- a/internal/cmd/self_update.go +++ b/internal/cmd/self_update.go @@ -13,6 +13,7 @@ import ( func newSelfUpdateCmd() *cobra.Command { var force bool + var allowUnsigned bool cmd := &cobra.Command{ Use: "self-update", @@ -26,18 +27,20 @@ If the daemon is running, it is automatically restarted so the new version is loaded into memory (otherwise heartbeat would keep reporting the old version until a manual restart).`, Example: ` unarr self-update - unarr self-update --force`, + unarr self-update --force + unarr self-update --allow-unsigned # accept releases missing checksums.txt.sig`, RunE: func(cmd *cobra.Command, args []string) error { - return runSelfUpdate(force) + return runSelfUpdate(force, allowUnsigned) }, } cmd.Flags().BoolVarP(&force, "force", "f", false, "reinstall even if already up to date") + cmd.Flags().BoolVar(&allowUnsigned, "allow-unsigned", false, "continue with SHA256-only verification when checksums.txt.sig is missing") return cmd } -func runSelfUpdate(force bool) error { +func runSelfUpdate(force, allowUnsigned bool) error { bold := color.New(color.Bold) green := color.New(color.FgGreen) yellow := color.New(color.FgYellow) @@ -74,6 +77,7 @@ func runSelfUpdate(force bool) error { upgrader := &upgrade.Upgrader{ CurrentVersion: currentClean, + AllowUnsigned: allowUnsigned, OnProgress: func(msg string) { fmt.Printf(" %s\n", msg) }, diff --git a/internal/cmd/upgrade.go b/internal/cmd/upgrade.go index c374603..63f56f9 100644 --- a/internal/cmd/upgrade.go +++ b/internal/cmd/upgrade.go @@ -7,6 +7,7 @@ import ( // newUpgradeCmd creates the `unarr upgrade` command as an alias for `self-update`. func newUpgradeCmd() *cobra.Command { var force bool + var allowUnsigned bool cmd := &cobra.Command{ Use: "upgrade", @@ -18,13 +19,15 @@ This is an alias for 'unarr self-update'. Checks GitHub for the latest release, verifies the checksum, and replaces the current binary. A backup is kept at .backup.`, Example: ` unarr upgrade - unarr upgrade --force`, + unarr upgrade --force + unarr upgrade --allow-unsigned`, RunE: func(cmd *cobra.Command, args []string) error { - return runSelfUpdate(force) + return runSelfUpdate(force, allowUnsigned) }, } cmd.Flags().BoolVarP(&force, "force", "f", false, "reinstall even if already up to date") + cmd.Flags().BoolVar(&allowUnsigned, "allow-unsigned", false, "continue with SHA256-only verification when checksums.txt.sig is missing") return cmd } diff --git a/internal/config/config.go b/internal/config/config.go index d3c18f9..7b0f6d7 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -49,6 +49,7 @@ type DownloadConfig struct { StallTimeout string `toml:"stall_timeout"` // e.g. "30m", "1h", "0" = unlimited (default: "30m") ListenPort int `toml:"listen_port"` // fixed port for incoming peer connections (default: 42069, 0 = random) StreamPort int `toml:"stream_port"` // fixed port for streaming HTTP server (default: 11818) + EnableUPnP bool `toml:"enable_upnp"` // map StreamPort to the WAN via UPnP/NAT-PMP (default: false; opt-in because it exposes the unauthenticated /stream + /hls endpoints to the public internet) WebRTC WebRTCConfig `toml:"webrtc"` Transcode TranscodeConfig `toml:"transcode"` } diff --git a/internal/engine/stream_server.go b/internal/engine/stream_server.go index 7440979..061d9e7 100644 --- a/internal/engine/stream_server.go +++ b/internal/engine/stream_server.go @@ -50,7 +50,12 @@ type StreamServer struct { url string // best single URL (backward compat) urls StreamURLs // all available URLs by network type upnpMapping *UPnPMapping - disableUPnP bool + // enableUPnP gates whether Listen() asks the gateway to publish the + // stream port to the WAN. UPnP is opt-in (false by default) because + // /stream and /hls have no auth — exposing them on the public internet + // would let any scanner enumerate active downloads. LAN and Tailscale + // access keep working without UPnP. + enableUPnP bool hls *HLSSessionRegistry // HLS sessions served on /hls//... @@ -65,10 +70,22 @@ type StreamServer struct { // NewStreamServer creates a stream server bound to the given port. // Call Listen() to start accepting connections, then SetFile() to serve content. +// +// UPnP is opt-in: call SetUPnPEnabled(true) before Listen() to publish the +// stream port on the WAN. Without it, only LAN and Tailscale clients can +// reach the server. This matches the security default — /stream and /hls +// have no auth, so exposing them to the public internet is something the +// operator must explicitly request. func NewStreamServer(port int) *StreamServer { return &StreamServer{port: port, hls: NewHLSSessionRegistry()} } +// SetUPnPEnabled toggles WAN publishing of the stream port. Call before +// Listen(); changes after Listen() are ignored for the active server. +func (ss *StreamServer) SetUPnPEnabled(enabled bool) { + ss.enableUPnP = enabled +} + // HLS returns the HLS session registry for this server. Daemon code uses it // to register a session when the backend asks for HLS playback. func (ss *StreamServer) HLS() *HLSSessionRegistry { return ss.hls } @@ -122,11 +139,16 @@ func (ss *StreamServer) Listen(ctx context.Context) error { if tsIP := TailscaleIP(); tsIP != "" { ss.urls.Tailscale = fmt.Sprintf("http://%s:%d/stream", tsIP, ss.port) } - if !ss.disableUPnP { - if mapping, err := SetupUPnP(ss.port); err == nil { + if ss.enableUPnP { + mapping, err := SetupUPnP(ss.port) + if err != nil { + log.Printf("[stream] UPnP setup failed: %v (only LAN/Tailscale clients will reach port %d)", err, ss.port) + } else { ss.upnpMapping = mapping ss.urls.Public = fmt.Sprintf("http://%s:%d/stream", mapping.ExternalIP, mapping.ExternalPort) } + } else { + log.Printf("[stream] UPnP disabled — port %d not published to WAN (set downloads.enable_upnp = true to opt in)", ss.port) } // Best single URL for backward compat: Tailscale > LAN > Public > localhost diff --git a/internal/engine/stream_server_test.go b/internal/engine/stream_server_test.go index 3c58b1a..2751749 100644 --- a/internal/engine/stream_server_test.go +++ b/internal/engine/stream_server_test.go @@ -384,8 +384,7 @@ func TestStreamServer_Health_WithFile(t *testing.T) { // nombre de fichero, taskID ni client IP cuando el caller no es loopback. // Protección contra reconnaissance vía LAN / UPnP / Tailscale. func TestStreamServer_Health_NonLoopback_NoLeak(t *testing.T) { - srv := NewStreamServer(0) - srv.disableUPnP = true + srv := NewStreamServer(0) // UPnP off by default — keep test hermetic ctx := context.Background() if err := srv.Listen(ctx); err != nil { t.Fatalf("Listen() error: %v", err) @@ -434,8 +433,7 @@ func TestStreamServer_Health_NonLoopback_NoLeak(t *testing.T) { // session IDs con caracteres ilegales devolviendo 404 (uniforme con sesión // inexistente) para no filtrar el formato aceptado a un attacker. func TestStreamServer_HLS_InvalidSessionID(t *testing.T) { - srv := NewStreamServer(0) - srv.disableUPnP = true + srv := NewStreamServer(0) // UPnP off by default — keep test hermetic ctx := context.Background() if err := srv.Listen(ctx); err != nil { t.Fatalf("Listen() error: %v", err) diff --git a/internal/engine/watch_reporter_test.go b/internal/engine/watch_reporter_test.go index bb7c7f5..ec69a71 100644 --- a/internal/engine/watch_reporter_test.go +++ b/internal/engine/watch_reporter_test.go @@ -185,8 +185,7 @@ func TestStreamServerByteTracking(t *testing.T) { t.Fatal(err) } - srv := NewStreamServer(0) - srv.disableUPnP = true + srv := NewStreamServer(0) // UPnP off by default — keep test hermetic ctx := context.Background() if err := srv.Listen(ctx); err != nil { t.Fatalf("listen: %v", err) diff --git a/internal/upgrade/download.go b/internal/upgrade/download.go index 1eaf577..b112b1d 100644 --- a/internal/upgrade/download.go +++ b/internal/upgrade/download.go @@ -2,6 +2,7 @@ package upgrade import ( "bufio" + "bytes" "context" "crypto/sha256" "encoding/hex" @@ -88,7 +89,23 @@ func download(ctx context.Context, version string) (string, error) { } // verifyChecksum downloads checksums.txt and verifies the archive's SHA256. +// When a release public key is embedded at build time (releasePubKeyBase64), +// the function also verifies an ed25519 signature over checksums.txt before +// trusting any hash inside it — this turns the checksum file from a passive +// integrity check into an authenticated artifact that a maintainer or CI key +// compromise cannot trivially forge. func verifyChecksum(ctx context.Context, version, archivePath string) error { + return verifyChecksumWithOptions(ctx, version, archivePath, true) +} + +// verifyChecksumOnly skips the ed25519 signature step. Used by Upgrader +// when --allow-unsigned is set and the release is known to predate signing +// (or when a release accidentally shipped without a .sig file). +func verifyChecksumOnly(ctx context.Context, version, archivePath string) error { + return verifyChecksumWithOptions(ctx, version, archivePath, false) +} + +func verifyChecksumWithOptions(ctx context.Context, version, archivePath string, verifySignature bool) error { // Download checksums.txt url := releaseURL(version, "checksums.txt") req, err := http.NewRequestWithContext(ctx, http.MethodGet, url, nil) @@ -107,11 +124,28 @@ func verifyChecksum(ctx context.Context, version, archivePath string) error { return fmt.Errorf("fetch checksums: HTTP %d", resp.StatusCode) } + // Read the entire checksums.txt content first so we can both parse and + // verify the signature over the same bytes. + checksumsContent, err := io.ReadAll(io.LimitReader(resp.Body, 1<<20)) + if err != nil { + return fmt.Errorf("read checksums: %w", err) + } + + // Verify ed25519 signature over checksums.txt before trusting its + // contents. Skipped silently when no key is embedded (handled by the + // caller via SignatureVerificationConfigured) or when the caller + // explicitly opts out via --allow-unsigned. + if verifySignature { + if err := verifyChecksumsSignature(ctx, version, checksumsContent); err != nil { + return fmt.Errorf("verify signature: %w", err) + } + } + // Parse checksums.txt — format: " " expectedName := archiveName(version) var expectedHash string - scanner := bufio.NewScanner(resp.Body) + scanner := bufio.NewScanner(bytes.NewReader(checksumsContent)) for scanner.Scan() { line := scanner.Text() parts := strings.Fields(line) diff --git a/internal/upgrade/signature.go b/internal/upgrade/signature.go new file mode 100644 index 0000000..cfcc93d --- /dev/null +++ b/internal/upgrade/signature.go @@ -0,0 +1,112 @@ +package upgrade + +import ( + "context" + "crypto/ed25519" + "encoding/base64" + "errors" + "fmt" + "io" + "net/http" + "strings" +) + +// releasePubKeyBase64 is the base64-encoded ed25519 public key used to verify +// `checksums.txt.sig` against `checksums.txt` during self-update. +// +// It is overridable at link time via ldflags so the same source compiles for +// users who do not yet have a release-signing keypair in their CI: +// +// -X github.com/torrentclaw/unarr/internal/upgrade.releasePubKeyBase64= +// +// When the variable is empty, signature verification is skipped and a warning +// is logged — checksum-only verification remains in force. This is the +// transitional default until the keypair is provisioned; flip to a non-empty +// value (and enable the corresponding CI signing step) to make signature +// verification mandatory. +var releasePubKeyBase64 = "" + +// ErrMissingSignature indicates the release does not ship a `.sig` file even +// though signature verification is required by an embedded public key. +var ErrMissingSignature = errors.New("release signature file is missing") + +// verifyChecksumsSignature downloads `checksums.txt.sig` (raw 64-byte ed25519 +// signature over the checksums.txt content) and verifies it with the embedded +// public key. Returns nil if verification succeeds or if no public key has +// been embedded yet (caller is expected to surface a warning in that case). +func verifyChecksumsSignature(ctx context.Context, version string, checksumsContent []byte) error { + pubKey, err := loadReleasePubKey() + if err != nil { + return fmt.Errorf("load release pubkey: %w", err) + } + if pubKey == nil { + // Signature verification not configured; caller decides what to do. + return nil + } + + url := releaseURL(version, "checksums.txt.sig") + req, err := http.NewRequestWithContext(ctx, http.MethodGet, url, nil) + if err != nil { + return err + } + req.Header.Set("User-Agent", "unarr-updater") + resp, err := httpClient.Do(req) + if err != nil { + return fmt.Errorf("fetch signature: %w", err) + } + defer resp.Body.Close() + if resp.StatusCode == http.StatusNotFound { + return ErrMissingSignature + } + if resp.StatusCode != http.StatusOK { + return fmt.Errorf("fetch signature: HTTP %d", resp.StatusCode) + } + + // Signature file is base64(signature)\n — small and bounded. + rawSig, err := io.ReadAll(io.LimitReader(resp.Body, 8*1024)) + if err != nil { + return fmt.Errorf("read signature: %w", err) + } + sig, err := decodeSignature(rawSig) + if err != nil { + return fmt.Errorf("decode signature: %w", err) + } + if len(sig) != ed25519.SignatureSize { + return fmt.Errorf("signature size %d, expected %d", len(sig), ed25519.SignatureSize) + } + if !ed25519.Verify(pubKey, checksumsContent, sig) { + return errors.New("ed25519 signature verification failed") + } + return nil +} + +// SignatureVerificationConfigured reports whether the build has a release +// public key embedded. The CLI surfaces this so users running a non-signed +// build get a clear warning rather than silent trust. +func SignatureVerificationConfigured() bool { + pubKey, err := loadReleasePubKey() + return err == nil && pubKey != nil +} + +func loadReleasePubKey() (ed25519.PublicKey, error) { + v := strings.TrimSpace(releasePubKeyBase64) + if v == "" { + return nil, nil + } + raw, err := base64.StdEncoding.DecodeString(v) + if err != nil { + return nil, fmt.Errorf("base64 decode: %w", err) + } + if len(raw) != ed25519.PublicKeySize { + return nil, fmt.Errorf("pubkey size %d, expected %d", len(raw), ed25519.PublicKeySize) + } + return ed25519.PublicKey(raw), nil +} + +// decodeSignature parses the base64-encoded signature emitted by +// scripts/sign-checksums (always base64 + trailing newline). A single +// expected format keeps the surface area minimal — a stricter parser is +// less likely to accept a hostile mirror's coincidentally-sized payload. +func decodeSignature(raw []byte) ([]byte, error) { + return base64.StdEncoding.DecodeString(strings.TrimSpace(string(raw))) +} diff --git a/internal/upgrade/signature_test.go b/internal/upgrade/signature_test.go new file mode 100644 index 0000000..eb85b68 --- /dev/null +++ b/internal/upgrade/signature_test.go @@ -0,0 +1,134 @@ +package upgrade + +import ( + "context" + "crypto/ed25519" + "crypto/rand" + "encoding/base64" + "errors" + "fmt" + "net/http" + "net/http/httptest" + "strings" + "testing" +) + +// withReleasePubKey temporarily swaps the embedded release public key and +// restores the previous value on test exit. +func withReleasePubKey(t *testing.T, encoded string) { + t.Helper() + prev := releasePubKeyBase64 + releasePubKeyBase64 = encoded + t.Cleanup(func() { releasePubKeyBase64 = prev }) +} + +func TestSignatureVerificationDisabledByDefault(t *testing.T) { + withReleasePubKey(t, "") + if SignatureVerificationConfigured() { + t.Fatal("expected SignatureVerificationConfigured() to be false when pubkey is empty") + } + // verifyChecksumsSignature should be a no-op when no key is embedded. + if err := verifyChecksumsSignature(context.Background(), "0.0.0", []byte("anything")); err != nil { + t.Fatalf("expected nil when pubkey is empty, got %v", err) + } +} + +func TestSignatureRejectsMalformedPubKey(t *testing.T) { + withReleasePubKey(t, "not-base64!!") + if _, err := loadReleasePubKey(); err == nil { + t.Fatal("expected error from malformed base64") + } +} + +func TestSignatureRejectsWrongSizePubKey(t *testing.T) { + withReleasePubKey(t, base64.StdEncoding.EncodeToString([]byte("too-short"))) + if _, err := loadReleasePubKey(); err == nil { + t.Fatal("expected error from wrong-size pubkey") + } +} + +func TestSignatureVerifiesGoodSignature(t *testing.T) { + pub, priv, err := ed25519.GenerateKey(rand.Reader) + if err != nil { + t.Fatalf("generate keypair: %v", err) + } + withReleasePubKey(t, base64.StdEncoding.EncodeToString(pub)) + + checksumsBody := []byte("deadbeef unarr_0.0.0_linux_amd64.tar.gz\n") + signature := ed25519.Sign(priv, checksumsBody) + + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if !strings.HasSuffix(r.URL.Path, "checksums.txt.sig") { + http.NotFound(w, r) + return + } + fmt.Fprintln(w, base64.StdEncoding.EncodeToString(signature)) + })) + defer srv.Close() + + prevHost := githubReleaseHost + githubReleaseHost = srv.URL + t.Cleanup(func() { githubReleaseHost = prevHost }) + + if err := verifyChecksumsSignature(context.Background(), "0.0.0", checksumsBody); err != nil { + t.Fatalf("verifyChecksumsSignature(good) = %v, want nil", err) + } +} + +func TestSignatureRejectsBadSignature(t *testing.T) { + pub, _, err := ed25519.GenerateKey(rand.Reader) + if err != nil { + t.Fatalf("generate keypair: %v", err) + } + withReleasePubKey(t, base64.StdEncoding.EncodeToString(pub)) + + // Sign with a DIFFERENT private key — should be rejected. + _, other, _ := ed25519.GenerateKey(rand.Reader) + body := []byte("checksum-line\n") + badSig := ed25519.Sign(other, body) + + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + fmt.Fprintln(w, base64.StdEncoding.EncodeToString(badSig)) + })) + defer srv.Close() + + prevHost := githubReleaseHost + githubReleaseHost = srv.URL + t.Cleanup(func() { githubReleaseHost = prevHost }) + + err = verifyChecksumsSignature(context.Background(), "0.0.0", body) + if err == nil || !strings.Contains(err.Error(), "verification failed") { + t.Fatalf("expected verification failure, got %v", err) + } +} + +func TestSignatureMissingFile(t *testing.T) { + pub, _, _ := ed25519.GenerateKey(rand.Reader) + withReleasePubKey(t, base64.StdEncoding.EncodeToString(pub)) + + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + http.NotFound(w, r) + })) + defer srv.Close() + prevHost := githubReleaseHost + githubReleaseHost = srv.URL + t.Cleanup(func() { githubReleaseHost = prevHost }) + + err := verifyChecksumsSignature(context.Background(), "0.0.0", []byte("body")) + if !errors.Is(err, ErrMissingSignature) { + t.Fatalf("expected ErrMissingSignature, got %v", err) + } +} + +func TestDecodeSignatureRejectsRaw(t *testing.T) { + // 64-byte payload that happens NOT to be valid base64 must error rather + // than be silently accepted as a raw signature — the only legitimate + // shape is base64-encoded text. + raw := make([]byte, ed25519.SignatureSize) + for i := range raw { + raw[i] = 0xff + } + if _, err := decodeSignature(raw); err == nil { + t.Fatal("expected error from non-base64 64-byte payload") + } +} diff --git a/internal/upgrade/upgrade.go b/internal/upgrade/upgrade.go index 6a675d2..6470ef1 100644 --- a/internal/upgrade/upgrade.go +++ b/internal/upgrade/upgrade.go @@ -13,6 +13,7 @@ package upgrade import ( "context" + "errors" "fmt" "log" "os" @@ -43,6 +44,13 @@ type Upgrader struct { CurrentVersion string // OnProgress is called with status messages during the upgrade process. OnProgress func(msg string) + // AllowUnsigned downgrades a missing checksums.txt.sig to a warning and + // continues with SHA256-only verification. Required to downgrade to a + // release published before signing was introduced, or to recover from + // an accidental release where the workflow's signing step was skipped. + // Default false — signature missing is a hard failure when a public + // key is embedded. + AllowUnsigned bool } func (u *Upgrader) log(msg string) { @@ -89,10 +97,21 @@ func (u *Upgrader) Execute(ctx context.Context, targetVersion string) Result { } defer os.Remove(archivePath) - // 5. Verify checksum - u.log("Verifying checksum...") + // 5. Verify checksum (and signature, if configured) + if SignatureVerificationConfigured() { + u.log("Verifying checksum + ed25519 signature...") + } else { + u.log("Verifying checksum (release signature verification not configured for this build)...") + } if err := verifyChecksum(ctx, targetVersion, archivePath); err != nil { - return u.fail("checksum: %v", err) + if errors.Is(err, ErrMissingSignature) && u.AllowUnsigned { + u.log("WARNING: release is unsigned and --allow-unsigned was passed; continuing with SHA256-only verification") + if err := verifyChecksumOnly(ctx, targetVersion, archivePath); err != nil { + return u.fail("checksum: %v", err) + } + } else { + return u.fail("checksum: %v", err) + } } // 6. Extract binary @@ -224,7 +243,12 @@ func archiveName(version string) string { return fmt.Sprintf("%s_%s_%s_%s.%s", binaryName, version, runtime.GOOS, runtime.GOARCH, ext) } +// githubReleaseHost is the base URL used to build release asset URLs. Exposed +// as a var (not a const) so tests can point it at an httptest.Server without +// touching production behaviour. +var githubReleaseHost = "https://github.com" + // releaseURL returns the download URL for a release asset. func releaseURL(version, filename string) string { - return fmt.Sprintf("https://github.com/%s/releases/download/v%s/%s", githubRepo, version, filename) + return fmt.Sprintf("%s/%s/releases/download/v%s/%s", githubReleaseHost, githubRepo, version, filename) } diff --git a/scripts/gen-release-key/main.go b/scripts/gen-release-key/main.go new file mode 100644 index 0000000..51dfbda --- /dev/null +++ b/scripts/gen-release-key/main.go @@ -0,0 +1,37 @@ +// gen-release-key generates an ed25519 keypair for signing release artifacts. +// Run once per repository, then store the printed values: +// +// RELEASE_SIGNING_KEY → GitHub Actions secret (private key, base64) +// RELEASE_SIGNING_PUBKEY → GitHub Actions variable (public key, base64) +// +// The public key is injected into the binary at build time via the +// goreleaser ldflags entry that resolves +// `github.com/torrentclaw/unarr/internal/upgrade.releasePubKeyBase64`. +// The private key is used by the workflow's "Sign checksums.txt" step. +// +// Build and run: +// +// go run ./scripts/gen-release-key +package main + +import ( + "crypto/ed25519" + "crypto/rand" + "encoding/base64" + "fmt" +) + +func main() { + pub, priv, err := ed25519.GenerateKey(rand.Reader) + if err != nil { + panic(err) + } + fmt.Println("# Add the following to your GitHub repository:") + fmt.Println("# - Settings → Secrets and variables → Actions → New repository secret") + fmt.Println("# RELEASE_SIGNING_KEY = ") + fmt.Println("# - Settings → Secrets and variables → Actions → New repository variable") + fmt.Println("# RELEASE_SIGNING_PUBKEY = ") + fmt.Println() + fmt.Printf("PUBLIC_KEY_BASE64=%s\n", base64.StdEncoding.EncodeToString(pub)) + fmt.Printf("PRIVATE_KEY_BASE64=%s\n", base64.StdEncoding.EncodeToString(priv)) +} diff --git a/scripts/sign-checksums/main.go b/scripts/sign-checksums/main.go new file mode 100644 index 0000000..0f95f05 --- /dev/null +++ b/scripts/sign-checksums/main.go @@ -0,0 +1,60 @@ +// sign-checksums signs the dist/checksums.txt file with an ed25519 private +// key and writes the base64-encoded signature to the path given by -out. +// +// Usage (from release workflow): +// +// go run ./scripts/sign-checksums \ +// -key "$RELEASE_SIGNING_KEY" \ +// -in dist/checksums.txt \ +// -out dist/checksums.txt.sig +// +// The companion CLI verifier (internal/upgrade/signature.go) requires the +// signature to be base64 text, so emitting base64 + trailing newline makes +// the artifact safe to inspect with `cat` / the GitHub release UI. +package main + +import ( + "crypto/ed25519" + "encoding/base64" + "flag" + "fmt" + "os" +) + +func main() { + keyB64 := flag.String("key", "", "base64-encoded ed25519 private key (PrivateKeySize = 64 bytes)") + in := flag.String("in", "", "path to file to sign") + out := flag.String("out", "", "path to write the base64-encoded signature") + flag.Parse() + + if *keyB64 == "" || *in == "" || *out == "" { + fmt.Fprintln(os.Stderr, "usage: sign-checksums -key -in -out ") + os.Exit(2) + } + + keyBytes, err := base64.StdEncoding.DecodeString(*keyB64) + if err != nil { + fail("decode key: %v", err) + } + if len(keyBytes) != ed25519.PrivateKeySize { + fail("private key size %d, expected %d", len(keyBytes), ed25519.PrivateKeySize) + } + priv := ed25519.PrivateKey(keyBytes) + + content, err := os.ReadFile(*in) + if err != nil { + fail("read input: %v", err) + } + + sig := ed25519.Sign(priv, content) + encoded := base64.StdEncoding.EncodeToString(sig) + "\n" + if err := os.WriteFile(*out, []byte(encoded), 0o644); err != nil { + fail("write signature: %v", err) + } + fmt.Printf("Signed %s (%d bytes) → %s\n", *in, len(content), *out) +} + +func fail(format string, args ...any) { + fmt.Fprintf(os.Stderr, format+"\n", args...) + os.Exit(1) +} From 060a3e48db066d7b61056370f68b09728de0d9cf Mon Sep 17 00:00:00 2001 From: Deivid Soto Date: Fri, 15 May 2026 18:48:59 +0200 Subject: [PATCH 03/11] fix(security): CORS allowlist, URL scheme guard, state perms, ZIP slip, mirror docs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Phase 3 security audit follow-up. Medium and low-severity hardenings plus a deferred-work plan for the cross-repo stream-token rollout. Stream server CORS: replace the wildcard Access-Control-Allow-Origin with an allowlist that echoes back only torrentclaw.com, app.torrentclaw.com, the local Next dev port (3030 — matches the web repo package.json) and any extras the operator adds via the new downloads.cors_extra_origins TOML key. A Vary: Origin header is now emitted whenever the request carries an Origin header so an intermediate cache cannot serve a stale ACAO to a different origin. URL scheme guard: openBrowser and OpenPlayer refuse any URL that is not http(s). Combined with passing the URL after "--" wherever the launched helper supports it (open, mpv, vlc, cvlc), this stops a leading "-" from being parsed as a switch by the spawned process. State file permissions: WriteState now writes 0o600 so the agent ID, PID and counters cannot be enumerated by another local user on a shared host. Matches the existing config file mode. ZIP slip defense-in-depth: extractZip extracts the safety check into safeZipPath, which canonicalises the entry name (normalising backslashes to "/"), rejects "..", "../" prefix and "/../" interior components, and verifies the final destination stays inside destDir before opening any file. Mirror fallback: documented the design for multi-provider mirrors.json hosting in the comment block on DefaultStaticFallbackURLs and added a follow-up note about signing it with the same ed25519 release key. The list is kept at one provider until the second host is provisioned and added to torrentclaw-web's STATIC_FALLBACKS. Deferred work: a new plan document Docs/plans/security-stream-token.md covers the per-task stream token (Phase 2.2 of the original audit) which requires coordinated web + CLI work and ships separately. --- Docs/plans/security-stream-token.md | 131 ++++++++++++++++++++++++++ internal/agent/mirror_client.go | 12 ++- internal/agent/state.go | 8 +- internal/cmd/daemon.go | 1 + internal/cmd/helpers.go | 17 +++- internal/cmd/helpers_test.go | 26 +++++ internal/config/config.go | 1 + internal/engine/stream_player.go | 28 +++++- internal/engine/stream_server.go | 90 ++++++++++++------ internal/engine/stream_server_test.go | 65 +++++++++++++ internal/engine/validate.go | 36 +++++++ internal/upgrade/extract.go | 58 ++++++++++-- internal/upgrade/upgrade_test.go | 37 ++++++++ 13 files changed, 462 insertions(+), 48 deletions(-) create mode 100644 Docs/plans/security-stream-token.md diff --git a/Docs/plans/security-stream-token.md b/Docs/plans/security-stream-token.md new file mode 100644 index 0000000..1a08e21 --- /dev/null +++ b/Docs/plans/security-stream-token.md @@ -0,0 +1,131 @@ +# Phase 2.2 — Per-task stream token (deferred) + +Status: deferred. Requires coordinated change in the web app +(`torrentclaw-web`) and the CLI daemon. Pulled out of the Phase 2 +security pass because the CLI-only fixes (UPnP opt-in, SSE caps, +signed self-update) ship without web-side work; the stream-token +work cannot. + +## Problem + +`/stream`, `/playlist.m3u` and `/hls//...` on the daemon +HTTP server have no authentication. Today, anyone who can reach the +listener and guesses (or learns) the `taskID` (for `/stream`) or +`sessionID` (for `/hls`) can fetch the active file. + +Mitigations already in place after Phase 1+2: + +- `sessionID` is restricted to a safe regex and is a server-issued + UUID v4 (122-bit entropy, not enumerable in practice). +- `/health` no longer leaks the active filename, taskID prefix or + client IP to remote callers (loopback diagnostics preserved). +- UPnP is opt-in, so by default the daemon is not exposed to the + public internet. +- The web client probes `/health` to pick LAN vs Tailscale. + +Residual risk: + +- On a shared LAN (open Wi-Fi, office network, dorm) any device can + reach the listener and brute-force `?id=` against + `/stream`. taskIDs are also UUIDs, so this is high entropy, but + the URL may leak through browser history, sharing, screen capture + or a passive logger and there is no second factor. +- A user who explicitly opts into UPnP exposes the same surface to + the entire internet. + +A per-task secret carried in the URL closes this without breaking +the `