From 433e375defea7e3666c6cb5e837494fa6945134f Mon Sep 17 00:00:00 2001 From: Deivid Soto Date: Fri, 15 May 2026 17:29:22 +0200 Subject: [PATCH] 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) +}