From edd188488c34d787bc023eb6b149970482548776 Mon Sep 17 00:00:00 2001 From: ApolloF <98666317+ApolloF@users.noreply.github.com> Date: Sun, 25 Oct 2026 16:05:23 +0100 Subject: [PATCH 1/2] Install only updates signed with the release key CI signs SHA256SUMS.txt together with the release tag using an ed25519 key from the RELEASE_SIGNING_KEY secret, and checks it against the key built into the app. The app installs only a Syncer.exe whose hash is in a validly signed sums file of a newer release, and never while a backup or an account change is running. --- .github/workflows/build.yml | 31 +++++ RELEASING.md | 18 ++- internal/app/update.go | 48 ++++++- internal/app/updateswap_test.go | 67 ++++++++++ internal/update/export_test.go | 10 ++ internal/update/pubkey.go | 23 ++++ internal/update/signed_test.go | 213 ++++++++++++++++++++++++++++++++ internal/update/update.go | 126 +++++++++++++++---- internal/update/update_test.go | 56 +-------- internal/update/verify.go | 56 +++++++++ tools/release-sign/main.go | 169 +++++++++++++++++++++++++ tools/release-sign/main_test.go | 110 +++++++++++++++++ 12 files changed, 842 insertions(+), 85 deletions(-) create mode 100644 internal/app/updateswap_test.go create mode 100644 internal/update/export_test.go create mode 100644 internal/update/pubkey.go create mode 100644 internal/update/signed_test.go create mode 100644 internal/update/verify.go create mode 100644 tools/release-sign/main.go create mode 100644 tools/release-sign/main_test.go diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 1475918..2dc611e 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -93,6 +93,16 @@ jobs: id-token: write # sign the provenance attestations: write steps: + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + with: + persist-credentials: false + + - uses: actions/setup-go@b7ad1dad31e06c5925ef5d2fc7ad053ef454303e # v7.0.0 + with: + go-version: '1.26.x' + check-latest: true + cache: false # this job holds the signing key; no restored cache may run in it + - uses: actions/download-artifact@d3f86a106a0bac45b974a628896c90dbdf5c8093 # v4.3.0 with: name: Syncer @@ -102,6 +112,26 @@ jobs: working-directory: dist run: sha256sum Syncer.exe Syncer-amd64-installer.exe | tee SHA256SUMS.txt + - name: Sign the checksums + # The app installs only an exe whose hash is in a sums file signed with + # this key (see RELEASING.md); a release without it must not be made. + env: + RELEASE_SIGNING_KEY: ${{ secrets.RELEASE_SIGNING_KEY }} + run: | + if [ -z "$RELEASE_SIGNING_KEY" ]; then + echo "::error::The RELEASE_SIGNING_KEY secret is empty." + exit 1 + fi + go run ./tools/release-sign -sign dist/SHA256SUMS.txt -tag "$GITHUB_REF_NAME" -key-env RELEASE_SIGNING_KEY + + - name: Check the signature against the key in the app + # Fails when the secret doesn't belong to the public key in pubkey.go + # (or that is still the placeholder): such a release would install nowhere. + # A key-change release is signed with the old key: RELEASING.md says how. + env: + PUBKEY_FILE: ${{ vars.RELEASE_PUBKEY_FILE }} + run: go run ./tools/release-sign -verify dist/SHA256SUMS.txt -tag "$GITHUB_REF_NAME" -pubfile "${PUBKEY_FILE:-internal/update/pubkey.go}" + - uses: actions/attest-build-provenance@e8998f949152b193b063cb0ec769d69d929409be # v2.4.0 with: subject-path: | @@ -121,3 +151,4 @@ jobs: dist/Syncer-amd64-installer.exe dist/Syncer.exe dist/SHA256SUMS.txt + dist/SHA256SUMS.txt.sig diff --git a/RELEASING.md b/RELEASING.md index 277efef..c68db10 100644 --- a/RELEASING.md +++ b/RELEASING.md @@ -4,9 +4,10 @@ Installed copies update themselves from the newest *published* GitHub release, s 1. Make sure `main` is green and has what you want to ship. 2. Tag it and push the tag: `git tag v1.2.0 && git push origin v1.2.0`. Use `v1.2.0-rc1` for a release candidate (it is marked as a pre-release and is never offered by the updater). -3. CI (`build`) tests and builds both files, then creates a **draft** release with `Syncer-amd64-installer.exe`, `Syncer.exe` and `SHA256SUMS.txt`, and records build provenance for the two exes. +3. CI (`build`) tests and builds both files, then signs `SHA256SUMS.txt` with the release key, and creates a **draft** release with `Syncer-amd64-installer.exe`, `Syncer.exe`, `SHA256SUMS.txt` and `SHA256SUMS.txt.sig`, and records build provenance for the two exes. 4. Download the draft's assets and check them: - `sha256sum -c SHA256SUMS.txt` (or `Get-FileHash`) matches the files. + - `go run ./tools/release-sign -verify SHA256SUMS.txt -tag v1.2.0 -pubfile internal/update/pubkey.go` accepts the signature (CI runs the same check on the signature it makes, and fails the release if it doesn't pass). Installed copies only update to a release that passes this. - `gh attestation verify Syncer.exe --repo ApolloF/syncer` and the same for the installer. 5. Smoke-test on a clean VM or PC: a fresh install from the installer, and an upgrade over the previous release (settings and backups still there, the app restarts into the new version). 6. Edit the release notes if needed, then publish the draft. A release candidate stays a pre-release. @@ -14,6 +15,21 @@ Installed copies update themselves from the newest *published* GitHub release, s If a published release is bad, mark it as a draft again (or delete it) so the updater stops offering it, then fix forward with a new tag. +## Release signing key + +Releases are signed so that a tampered or replaced asset is never installed: the app embeds the public key and installs only a Syncer.exe whose hash is in a validly signed SHA256SUMS.txt. The private key is the base64 of an ed25519 seed, one line. + +One-time setup: + +1. `go run ./tools/release-sign -keygen -out ` writes the private key (it refuses to overwrite) and prints only the public key. +2. `Get-Content -Raw | gh secret set RELEASE_SIGNING_KEY --repo ApolloF/Syncer` gives it to CI (PowerShell has no `<`; in bash use `gh secret set RELEASE_SIGNING_KEY < `). Don't use `--body`: the key would land in the process arguments and the shell history. The release job fails when the secret is empty. +3. Paste the printed public key into `releasePublicKey` in `internal/update/pubkey.go` and release that. Until then no update verifies, so none is installed. +4. Keep the private key file somewhere safe and offline, and delete the copy on the machine you generated it on. The file mode 0600 does nothing on Windows, so until it is offline keep it in a folder under your user profile. + +Recommended: move the secret into a GitHub environment that only tags `v*` may deploy to (repository settings), so no other branch's workflow can read it. + +If the key is lost or leaked: make a new one as above, put its public key in `pubkey.go`, and publish that release signed with the old key, so installed copies accept it and from then on trust the new key. CI checks each signature against the key in `pubkey.go`, which for the bridge release is the new one, so for that one tag keep a copy of the old `pubkey.go` in the repo and set the repository variable `RELEASE_PUBKEY_FILE` to its path; remove the variable afterwards. Switch the `RELEASE_SIGNING_KEY` secret to the new key only after the bridge release is out. A leaked key can't be revoked for copies that haven't updated yet: until they take the bridge release, whoever has the key can sign updates for them. If the old key is lost, nothing signed with a new key will be accepted by installed copies: users must download and install that release by hand. + ## Personal build `wails build -tags personal` makes the maintainer's build (see [CONTRIBUTING.md](CONTRIBUTING.md#builds-public-vs-personal)). It is built locally, with the Google client passed through `-ldflags`, and is never published or attached to a release. CI only builds the public one. diff --git a/internal/app/update.go b/internal/app/update.go index 156e247..0cdf637 100644 --- a/internal/app/update.go +++ b/internal/app/update.go @@ -16,6 +16,8 @@ import ( "golang.org/x/sys/windows" "golang.org/x/sys/windows/registry" + "github.com/ApolloF/syncer/internal/accounts" + "github.com/ApolloF/syncer/internal/backup" "github.com/ApolloF/syncer/internal/features" "github.com/ApolloF/syncer/internal/logx" "github.com/ApolloF/syncer/internal/meta" @@ -127,7 +129,31 @@ func (a *App) InstallUpdate() error { return a.installUpdate(ctx, false) } -var errBackupRunning = errors.New("a backup is running; update once it has finished") +// errBusy: Syncer is backing up, or moving saves between accounts, and the +// exe swap must not interrupt that. +var errBusy = errors.New("Syncer is busy with your saves; update once that has finished") + +// swapExe puts the downloaded exe in place while no backup runs and no +// account operation is pending or starting. +func swapExe(exe, downloaded string) error { + // A running operation holds the lock for a while; say so at once instead + // of waiting for it. + if _, pending := accounts.PendingOp(); pending { + return errBusy + } + err := accounts.Locked(func() error { + unlock, err := backup.Lock() + if err != nil { + return errBusy + } + defer unlock() + return update.Replace(exe, downloaded) + }) + if errors.Is(err, accounts.ErrBusy) { + return errBusy + } + return err +} // installUpdate replaces Syncer.exe with the newest release's and restarts. // Installed and portable copies both update this way (the installer does @@ -146,9 +172,9 @@ func (a *App) installUpdate(ctx context.Context, auto bool) error { a.mu.Unlock() return errors.New("Syncer is already updating") } - if a.backingUp { + if _, pending := accounts.PendingOp(); a.backingUp || pending { a.mu.Unlock() - return errBackupRunning + return errBusy } a.updating = true a.mu.Unlock() @@ -168,7 +194,11 @@ func (a *App) installUpdate(ctx context.Context, auto bool) error { if r.Exe == nil { return errors.New("Syncer " + r.Tag + " can't be installed from here; download it from GitHub") } - path, err := update.Download(ctx, r.Exe, filepath.Dir(exe)) + path, err := update.Download(ctx, r, version, filepath.Dir(exe)) + if errors.Is(err, update.ErrNotSigned) { + logx.Printf("Syncer %s was not installed: %v", r.Tag, err) + return update.ErrNotSigned + } if err != nil { return fmt.Errorf("download Syncer %s: %w", r.Tag, err) } @@ -180,10 +210,16 @@ func (a *App) installUpdate(ctx context.Context, auto bool) error { if auto { return nil // next time } - return errBackupRunning + return errBusy } - if err := update.Replace(exe, path); err != nil { + if err := swapExe(exe, path); err != nil { _ = os.Remove(path) + if errors.Is(err, errBusy) { + if auto { + return nil // next time + } + return err + } return fmt.Errorf("install Syncer %s: %w", r.Tag, err) } logx.Printf("updated to Syncer %s (from %s), restarting", r.Tag, version) diff --git a/internal/app/updateswap_test.go b/internal/app/updateswap_test.go new file mode 100644 index 0000000..f4716e4 --- /dev/null +++ b/internal/app/updateswap_test.go @@ -0,0 +1,67 @@ +package app + +import ( + "errors" + "os" + "path/filepath" + "testing" + + "github.com/ApolloF/syncer/internal/backup" + "github.com/ApolloF/syncer/internal/features" + "github.com/ApolloF/syncer/internal/paths" +) + +func TestSwapExeWaitsForBackupsAndAccountOperations(t *testing.T) { + if features.Personal { + t.Skip("the personal build never swaps the exe") + } + t.Cleanup(paths.SetRootForTest(paths.Roaming, t.TempDir())) + dir := t.TempDir() + exe, next := filepath.Join(dir, "Syncer.exe"), filepath.Join(dir, "Syncer.exe.new") + write := func(p, s string) { + t.Helper() + if err := os.WriteFile(p, []byte(s), 0o644); err != nil { + t.Fatal(err) + } + } + write(exe, "old") + write(next, "new") + untouched := func(when string) { + t.Helper() + if b, _ := os.ReadFile(exe); string(b) != "old" { + t.Fatalf("%s: exe = %q, was replaced", when, b) + } + } + + unlock, err := backup.Lock() + if err != nil { + t.Fatal(err) + } + if err := swapExe(exe, next); !errors.Is(err, errBusy) { + t.Fatalf("during a backup: %v, want errBusy", err) + } + untouched("during a backup") + unlock() + + journal := filepath.Join(paths.AppDir(), "accounts-op.json") + write(journal, `{"kind":"switch"}`) + if err := swapExe(exe, next); !errors.Is(err, errBusy) { + t.Fatalf("during an account operation: %v, want errBusy", err) + } + untouched("during an account operation") + if err := os.Remove(journal); err != nil { + t.Fatal(err) + } + + if err := swapExe(exe, next); err != nil { + t.Fatal(err) + } + if b, _ := os.ReadFile(exe); string(b) != "new" { + t.Fatalf("exe = %q after the swap", b) + } + if unlock, err := backup.Lock(); err != nil { + t.Fatalf("the backup lock was kept: %v", err) + } else { + unlock() + } +} diff --git a/internal/update/export_test.go b/internal/update/export_test.go new file mode 100644 index 0000000..5656001 --- /dev/null +++ b/internal/update/export_test.go @@ -0,0 +1,10 @@ +package update + +import "crypto/ed25519" + +// SetPublicKeyForTest makes updates verify against key; it returns the undo. +func SetPublicKeyForTest(key ed25519.PublicKey) (restore func()) { + prev := embeddedKey + embeddedKey = key + return func() { embeddedKey = prev } +} diff --git a/internal/update/pubkey.go b/internal/update/pubkey.go new file mode 100644 index 0000000..403296f --- /dev/null +++ b/internal/update/pubkey.go @@ -0,0 +1,23 @@ +package update + +import ( + "crypto/ed25519" + "encoding/base64" +) + +// releasePublicKey is the base64 ed25519 public key that releases are signed +// with (printed by `go run ./tools/release-sign -keygen -out `). While +// it is not a valid key, no update verifies, so none is installed. CI checks +// the signature it just made against this very line. +const releasePublicKey = "UNSET: paste the public key from release-sign -keygen" + +// embeddedKey is releasePublicKey decoded; nil when that isn't a valid key. +var embeddedKey = decodeKey(releasePublicKey) + +func decodeKey(s string) ed25519.PublicKey { + b, err := base64.StdEncoding.DecodeString(s) + if err != nil || len(b) != ed25519.PublicKeySize { + return nil + } + return b +} diff --git a/internal/update/signed_test.go b/internal/update/signed_test.go new file mode 100644 index 0000000..edfbb3b --- /dev/null +++ b/internal/update/signed_test.go @@ -0,0 +1,213 @@ +package update + +import ( + "context" + "crypto/ed25519" + "crypto/rand" + "crypto/sha256" + "encoding/base64" + "encoding/hex" + "errors" + "fmt" + "net/http" + "net/http/httptest" + "os" + "path/filepath" + "strings" + "testing" +) + +var testExe = []byte("new syncer") + +// served describes a fake release. Zero values mean a correctly signed v1.2.0. +type served struct { + key ed25519.PrivateKey + signTag string // tag the signature was made for + sigTag string // folder the signature is served from + tweak func(sums, sig *string, exe *[]byte) // changes what is served +} + +func newKey(t *testing.T) (ed25519.PublicKey, ed25519.PrivateKey) { + t.Helper() + pub, priv, err := ed25519.GenerateKey(rand.Reader) + if err != nil { + t.Fatal(err) + } + return pub, priv +} + +// release starts a server for s and returns what Latest finds there, with +// the download URLs pointed at it. +func release(t *testing.T, s served) Release { + t.Helper() + const tag = "v1.2.0" + if s.signTag == "" { + s.signTag = tag + } + if s.sigTag == "" { + s.sigTag = tag + } + sum := sha256.Sum256(testExe) + sums := hex.EncodeToString(sum[:]) + " Syncer.exe\n" + strings.Repeat("0", 64) + " Syncer-amd64-installer.exe\n" + sig := base64.StdEncoding.EncodeToString(ed25519.Sign(s.key, SignedMessage(s.signTag, []byte(sums)))) + exe := testExe + if s.tweak != nil { + s.tweak(&sums, &sig, &exe) + } + var srv *httptest.Server + srv = httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch r.URL.Path { + case "/latest": + assets := fmt.Sprintf(`{"name":"Syncer.exe","browser_download_url":"%[1]s/download/%[5]s/Syncer.exe","size":%[2]d,"digest":"sha256:%[3]s"}, + {"name":"SHA256SUMS.txt","browser_download_url":"%[1]s/download/%[5]s/SHA256SUMS.txt","size":%[4]d}`, + srv.URL, len(exe), hex.EncodeToString(sum[:]), len(sums), tag) + if sig != "" { + assets += fmt.Sprintf(`,{"name":"SHA256SUMS.txt.sig","browser_download_url":"%s/download/%s/SHA256SUMS.txt.sig","size":%d}`, srv.URL, s.sigTag, len(sig)) + } + fmt.Fprintf(w, `{"tag_name":%q,"html_url":%q,"assets":[%s]}`, tag, ReleasesURL+"tag/"+tag, assets) + case "/download/" + tag + "/Syncer.exe": + w.Write(exe) + case "/download/" + tag + "/SHA256SUMS.txt": + w.Write([]byte(sums)) + case "/download/" + s.sigTag + "/SHA256SUMS.txt.sig": + w.Write([]byte(sig)) + default: + http.NotFound(w, r) + } + })) + t.Cleanup(srv.Close) + prevL, prevD, prevP := latestURL, downloadURL, personal + t.Cleanup(func() { latestURL, downloadURL, personal = prevL, prevD, prevP }) + latestURL, downloadURL, personal = srv.URL+"/latest", srv.URL+"/download/", false + + r, err := Latest(context.Background()) + if err != nil { + t.Fatal(err) + } + return r +} + +func TestSignedReleaseDownloads(t *testing.T) { + pub, priv := newKey(t) + t.Cleanup(SetPublicKeyForTest(pub)) + r := release(t, served{key: priv}) + path, err := Download(context.Background(), r, "v1.1.0", t.TempDir()) + if err != nil { + t.Fatal(err) + } + if b, _ := os.ReadFile(path); string(b) != string(testExe) { + t.Fatalf("downloaded %q", b) + } +} + +func TestUnsignedReleasesAreNotInstalled(t *testing.T) { + pub, priv := newKey(t) + _, otherPriv := newKey(t) + for name, s := range map[string]served{ + "no signature": {key: priv, tweak: func(_, sig *string, _ *[]byte) { *sig = "" }}, + "bad signature": {key: priv, tweak: func(_, sig *string, _ *[]byte) { *sig = "AAAA" }}, + "another key": {key: otherPriv}, + "tampered sums": {key: priv, tweak: func(sums, _ *string, _ *[]byte) { *sums = strings.Replace(*sums, "0", "1", 1) }}, + "exe hash differs": {key: priv, tweak: func(_, _ *string, exe *[]byte) { *exe = []byte("evil syncer") }}, + "older release": {key: priv, signTag: "v1.1.0"}, + "signed for another tag": {key: priv, signTag: "v1.2.0+evil"}, + "mixed tags": {key: priv, sigTag: "v1.1.0"}, + } { + t.Run(name, func(t *testing.T) { + t.Cleanup(SetPublicKeyForTest(pub)) + r := release(t, s) + dir := t.TempDir() + path, err := Download(context.Background(), r, "v1.1.0", dir) + if !errors.Is(err, ErrNotSigned) || path != "" { + t.Fatalf("Download = %q, %v; want ErrNotSigned", path, err) + } + if entries, _ := os.ReadDir(dir); len(entries) != 0 { + t.Fatalf("files left in the folder: %v", entries) + } + }) + } +} + +func TestOnlyNewerReleasesDownload(t *testing.T) { + pub, priv := newKey(t) + t.Cleanup(SetPublicKeyForTest(pub)) + r := release(t, served{key: priv}) + for _, current := range []string{"v1.2.0", "v2.0.0"} { + if _, err := Download(context.Background(), r, current, t.TempDir()); err == nil { + t.Errorf("v1.2.0 downloaded over %s", current) + } + } +} + +func TestFilesMustComeFromTheReleasesOwnFolder(t *testing.T) { + pub, priv := newKey(t) + t.Cleanup(SetPublicKeyForTest(pub)) + r := release(t, served{key: priv}) + r.Exe.URL = strings.Replace(r.Exe.URL, "/v1.2.0/", "/v1.1.0/", 1) + if _, err := Download(context.Background(), r, "v1.0.0", t.TempDir()); !errors.Is(err, ErrNotSigned) { + t.Fatalf("Download = %v, want ErrNotSigned", err) + } +} + +func TestPlaceholderKeyNeverVerifies(t *testing.T) { + _, priv := newKey(t) + r := release(t, served{key: priv}) + if _, err := Download(context.Background(), r, "v1.1.0", t.TempDir()); !errors.Is(err, ErrNotSigned) { + t.Fatalf("Download with the unset release key = %v, want ErrNotSigned", err) + } +} + +func TestDigestMismatchIsNotInstalled(t *testing.T) { + pub, priv := newKey(t) + t.Cleanup(SetPublicKeyForTest(pub)) + r := release(t, served{key: priv}) + r.Exe.SHA256 = strings.Repeat("0", 64) + dir := t.TempDir() + if _, err := Download(context.Background(), r, "v1.1.0", dir); !errors.Is(err, ErrNotSigned) { + t.Fatalf("Download = %v, want ErrNotSigned", err) + } + if _, err := os.Stat(filepath.Join(dir, newName)); !os.IsNotExist(err) { + t.Fatal("a bad download was left behind") + } + r.Exe.URL = "https://example.com/Syncer.exe" + if _, err := Download(context.Background(), r, "v1.1.0", dir); err == nil { + t.Fatal("a download from outside the releases was accepted") + } +} + +func TestVerifyParsesAndIsStrict(t *testing.T) { + pub, priv := newKey(t) + h := strings.Repeat("ab", 32) + sign := func(s string) []byte { + return []byte(base64.StdEncoding.EncodeToString(ed25519.Sign(priv, SignedMessage("v1.2.0", []byte(s))))) + } + + good := h + " Syncer.exe\n" + h + " *Other.exe\n\n" + m, err := Verify("v1.2.0", []byte(good), sign(good), pub) + if s, o := m["Syncer.exe"], m["Other.exe"]; err != nil || len(m) != 2 || hex.EncodeToString(s[:]) != h || hex.EncodeToString(o[:]) != h { + t.Fatalf("Verify = %v, %v", m, err) + } + for name, s := range map[string]string{ + "duplicate": h + " A.exe\n" + h + " A.exe\n", + "bad hex": strings.Repeat("zz", 32) + " A.exe\n", + "short hash": "abcd A.exe\n", + "slash": h + " dir/A.exe\n", + "backslash": h + ` dir\A.exe` + "\n", + "one space": h + " A.exe\n", + "no name": h + " \n", + "no separator": h + "\n", + } { + if _, err := Verify("v1.2.0", []byte(s), sign(s), pub); !errors.Is(err, ErrNotSigned) { + t.Errorf("%s: Verify = %v, want ErrNotSigned", name, err) + } + } + if _, err := Verify("v1.2.0", []byte(good), sign(good), nil); !errors.Is(err, ErrNotSigned) { + t.Errorf("no key: Verify = %v", err) + } + if _, err := Verify("v1.2.0", []byte(good+"\n"), sign(good), pub); !errors.Is(err, ErrNotSigned) { + t.Errorf("changed sums: Verify = %v", err) + } + if _, err := Verify("v1.3.0", []byte(good), sign(good), pub); !errors.Is(err, ErrNotSigned) { + t.Errorf("another tag: Verify = %v", err) + } +} diff --git a/internal/update/update.go b/internal/update/update.go index 7dffb69..1e5f86a 100644 --- a/internal/update/update.go +++ b/internal/update/update.go @@ -26,7 +26,10 @@ const ReleasesURL = "https://github.com/ApolloF/syncer/releases/" const ( assetName = "Syncer.exe" + sumsName = "SHA256SUMS.txt" + sigName = sumsName + ".sig" maxSize = 200 << 20 + maxSmall = 64 << 10 // the sums and signature files are tiny ) // Variables for tests. @@ -42,16 +45,18 @@ var ( // Release is a published Syncer release. type Release struct { - Tag string // e.g. "v0.7.0" - URL string // release page - Exe *Asset // the release's Syncer.exe; nil when it has none (or no checksum) + Tag string // e.g. "v0.7.0" + URL string // release page + Exe *Asset // the release's Syncer.exe; nil when it has none + Sums *Asset // SHA256SUMS.txt, which the signature covers; nil when missing + Sig *Asset // SHA256SUMS.txt.sig; nil when missing (the release is unsigned) } // Asset is a file attached to a release. type Asset struct { URL string Size int64 - SHA256 string // hex, from GitHub's digest of the upload + SHA256 string // hex, from GitHub's digest of the upload; an extra check only, the signed sums decide } // Variables for tests. @@ -146,14 +151,25 @@ func decodeRelease(body io.Reader) (Release, error) { } rel := Release{Tag: r.Tag, URL: page} for _, a := range r.Assets { - sum, ok := strings.CutPrefix(a.Digest, "sha256:") - if a.Name != assetName || !strings.HasPrefix(a.URL, downloadURL) || a.Size <= 0 || a.Size > maxSize || !ok { + if !strings.HasPrefix(a.URL, downloadURL+r.Tag+"/") || a.Size <= 0 || a.Size > maxSize { continue } - if b, err := hex.DecodeString(sum); err != nil || len(b) != sha256.Size { - continue + asset := &Asset{URL: a.URL, Size: a.Size} + switch a.Name { + case assetName: + // GitHub's digest is optional: the signed sums are what counts. + if sum, ok := strings.CutPrefix(a.Digest, "sha256:"); ok { + if b, err := hex.DecodeString(sum); err != nil || len(b) != sha256.Size { + continue + } + asset.SHA256 = strings.ToLower(sum) + } + rel.Exe = asset + case sumsName: + rel.Sums = asset + case sigName: + rel.Sig = asset } - rel.Exe = &Asset{URL: a.URL, Size: a.Size, SHA256: strings.ToLower(sum)} } return rel, nil } @@ -163,31 +179,56 @@ func decodeRelease(body io.Reader) (Release, error) { // updating it would silently drop them. It is rebuilt from source instead. var ErrPersonalBuild = errors.New("this is the personal build: rebuild it from the source to update (wails build -tags personal)") -// Download fetches a into dir (next to the exe it replaces, so the swap is a -// rename on one volume) and checks it against its checksum. It returns the -// downloaded file. -func Download(ctx context.Context, a *Asset, dir string) (string, error) { +// Download fetches the release's Syncer.exe into dir (next to the exe it +// replaces, so the swap is a rename on one volume) and returns the file. The +// exe must be one the release's signed SHA256SUMS.txt names; a release +// without a valid signature, or whose exe doesn't match, fails with +// ErrNotSigned and leaves nothing behind. The release must be newer than +// current, and all its files must sit under its own tag's download folder. +func Download(ctx context.Context, r Release, current, dir string) (string, error) { if personal { return "", ErrPersonalBuild } + a := r.Exe if a == nil || !strings.HasPrefix(a.URL, downloadURL) { return "", errors.New("the release has no Syncer.exe to update from") } - ctx, cancel := context.WithTimeout(ctx, 10*time.Minute) - defer cancel() - req, err := http.NewRequestWithContext(ctx, http.MethodGet, a.URL, nil) + if !Newer(r.Tag, current) { + return "", errors.New("this is already the newest version") + } + if r.Sums == nil || r.Sig == nil { + return "", notSigned("the release has no signature") + } + for _, f := range []*Asset{a, r.Sums, r.Sig} { + if !strings.HasPrefix(f.URL, downloadURL+r.Tag+"/") { + return "", notSigned("the files aren't all from release " + r.Tag) + } + } + key := embeddedKey + sums, err := fetchSmall(ctx, r.Sums) if err != nil { return "", err } - req.Header.Set("User-Agent", "Syncer") - resp, err := http.DefaultClient.Do(req) + sig, err := fetchSmall(ctx, r.Sig) if err != nil { return "", err } - defer resp.Body.Close() - if resp.StatusCode != http.StatusOK { - return "", errors.New("GitHub answered " + resp.Status) + hashes, err := Verify(r.Tag, sums, sig, key) + if err != nil { + return "", err } + want, ok := hashes[assetName] + if !ok { + return "", notSigned(assetName + " isn't in the signed checksums") + } + + ctx, cancel := context.WithTimeout(ctx, 10*time.Minute) + defer cancel() + resp, err := get(ctx, a.URL) + if err != nil { + return "", err + } + defer resp.Body.Close() path := filepath.Join(dir, newName) f, err := os.Create(path) if err != nil { @@ -201,8 +242,11 @@ func Download(ctx context.Context, a *Asset, dir string) (string, error) { if err == nil && n != a.Size { err = errors.New("the download is incomplete") } - if err == nil && hex.EncodeToString(h.Sum(nil)) != a.SHA256 { - err = errors.New("the download doesn't match the release's checksum") + if err == nil && ([sha256.Size]byte)(h.Sum(nil)) != want { + err = notSigned("the download isn't the signed file") + } + if err == nil && a.SHA256 != "" && hex.EncodeToString(h.Sum(nil)) != a.SHA256 { + err = notSigned("the download doesn't match GitHub's checksum") } if err != nil { _ = os.Remove(path) @@ -211,6 +255,42 @@ func Download(ctx context.Context, a *Asset, dir string) (string, error) { return path, nil } +func get(ctx context.Context, url string) (*http.Response, error) { + req, err := http.NewRequestWithContext(ctx, http.MethodGet, url, nil) + if err != nil { + return nil, err + } + req.Header.Set("User-Agent", "Syncer") + resp, err := http.DefaultClient.Do(req) + if err != nil { + return nil, err + } + if resp.StatusCode != http.StatusOK { + resp.Body.Close() + return nil, errors.New("GitHub answered " + resp.Status) + } + return resp, nil +} + +// fetchSmall reads a release's sums or signature file. +func fetchSmall(ctx context.Context, a *Asset) ([]byte, error) { + ctx, cancel := context.WithTimeout(ctx, attemptTimeout) + defer cancel() + resp, err := get(ctx, a.URL) + if err != nil { + return nil, err + } + defer resp.Body.Close() + b, err := io.ReadAll(io.LimitReader(resp.Body, maxSmall+1)) + if err != nil { + return nil, err + } + if len(b) > maxSmall { + return nil, notSigned("the signature files are too big") + } + return b, nil +} + // Names of the new exe while it downloads, and of the replaced one until it // can be deleted (Windows lets a running exe be renamed, not deleted). const ( diff --git a/internal/update/update_test.go b/internal/update/update_test.go index f40c0c3..a4c52e3 100644 --- a/internal/update/update_test.go +++ b/internal/update/update_test.go @@ -2,8 +2,6 @@ package update import ( "context" - "crypto/sha256" - "encoding/hex" "errors" "fmt" "net/http" @@ -51,58 +49,6 @@ func TestNewer(t *testing.T) { } } -func TestLatestAndDownload(t *testing.T) { - exe := []byte("new syncer") - sum := sha256.Sum256(exe) - var srv *httptest.Server - srv = httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - switch r.URL.Path { - case "/latest": - fmt.Fprintf(w, `{"tag_name":"v1.2.0","html_url":%q,"assets":[ - {"name":"Syncer-amd64-installer.exe","browser_download_url":"%[2]s/download/v1.2.0/Syncer-amd64-installer.exe","size":5,"digest":"sha256:%[3]s"}, - {"name":"Syncer.exe","browser_download_url":"%[2]s/download/v1.2.0/Syncer.exe","size":%[4]d,"digest":"sha256:%[3]s"}]}`, - ReleasesURL+"tag/v1.2.0", srv.URL, hex.EncodeToString(sum[:]), len(exe)) - case "/download/v1.2.0/Syncer.exe": - w.Write(exe) - default: - http.NotFound(w, r) - } - })) - defer srv.Close() - defer func(l, d string, p bool) { latestURL, downloadURL, personal = l, d, p }(latestURL, downloadURL, personal) - latestURL, downloadURL, personal = srv.URL+"/latest", srv.URL+"/download/", false - - r, err := Latest(context.Background()) - if err != nil { - t.Fatal(err) - } - if r.Tag != "v1.2.0" || r.Exe == nil || r.Exe.URL != srv.URL+"/download/v1.2.0/Syncer.exe" { - t.Fatalf("Latest = %+v", r) - } - dir := t.TempDir() - path, err := Download(context.Background(), r.Exe, dir) - if err != nil { - t.Fatal(err) - } - if b, _ := os.ReadFile(path); string(b) != string(exe) { - t.Fatalf("downloaded %q", b) - } - - bad := *r.Exe - bad.SHA256 = strings.Repeat("0", 64) - if _, err := Download(context.Background(), &bad, dir); err == nil { - t.Fatal("a download with the wrong checksum was accepted") - } - if _, err := os.Stat(filepath.Join(dir, newName)); !os.IsNotExist(err) { - t.Fatal("a bad download was left behind") - } - bad = *r.Exe - bad.URL = "https://example.com/Syncer.exe" - if _, err := Download(context.Background(), &bad, dir); err == nil { - t.Fatal("a download from outside the releases was accepted") - } -} - func TestLatestRetries(t *testing.T) { defer func(l string, a, p time.Duration) { latestURL, attemptTimeout, retryPause = l, a, p }(latestURL, attemptTimeout, retryPause) attemptTimeout, retryPause = 200*time.Millisecond, time.Millisecond @@ -186,7 +132,7 @@ func TestPersonalBuildNeverInstalls(t *testing.T) { if err := os.WriteFile(exe, []byte("mine"), 0o644); err != nil { t.Fatal(err) } - if _, err := Download(context.Background(), &Asset{URL: downloadURL + "v9.9.9/Syncer.exe", Size: 1}, dir); !errors.Is(err, ErrPersonalBuild) { + if _, err := Download(context.Background(), Release{Exe: &Asset{URL: downloadURL + "v9.9.9/Syncer.exe", Size: 1}}, "v1.0.0", dir); !errors.Is(err, ErrPersonalBuild) { t.Errorf("Download err = %v, want ErrPersonalBuild", err) } if err := Replace(exe, filepath.Join(dir, newName)); !errors.Is(err, ErrPersonalBuild) { diff --git a/internal/update/verify.go b/internal/update/verify.go new file mode 100644 index 0000000..a01e540 --- /dev/null +++ b/internal/update/verify.go @@ -0,0 +1,56 @@ +package update + +import ( + "crypto/ed25519" + "crypto/sha256" + "encoding/base64" + "encoding/hex" + "errors" + "fmt" + "strings" +) + +// ErrNotSigned is what an update that can't be vouched for fails with: no +// signature, a wrong one, or an exe that isn't the one that was signed. +var ErrNotSigned = errors.New("This update isn't signed, so it wasn't installed.") + +func notSigned(why string) error { return fmt.Errorf("%w (%s)", ErrNotSigned, why) } + +// SignedMessage is what a release's signature covers: the tag too, so that +// an older release's signed files can't be passed off as a newer one. +func SignedMessage(tag string, sums []byte) []byte { + return append([]byte("Syncer "+tag+"\n"), sums...) +} + +// Verify checks sig (base64, as release-sign writes it) over tag and sums +// with pubKey and returns the SHA-256 of every file the sums name. sums is +// sha256sum's output: " " or " *" per line. +func Verify(tag string, sums, sig []byte, pubKey ed25519.PublicKey) (map[string][sha256.Size]byte, error) { + if len(pubKey) != ed25519.PublicKeySize { + return nil, notSigned("no valid release key") + } + raw, err := base64.StdEncoding.DecodeString(strings.TrimSpace(string(sig))) + if err != nil || !ed25519.Verify(pubKey, SignedMessage(tag, sums), raw) { + return nil, notSigned("bad signature") + } + out := map[string][sha256.Size]byte{} + for _, line := range strings.Split(string(sums), "\n") { + if line == "" { + continue + } + hexSum, rest, ok := strings.Cut(line, " ") + if !ok || len(rest) < 2 || (rest[0] != ' ' && rest[0] != '*') { + return nil, notSigned("malformed checksums") + } + name := rest[1:] + b, err := hex.DecodeString(hexSum) + if err != nil || len(b) != sha256.Size || name == "" || strings.ContainsAny(name, `/\`) { + return nil, notSigned("malformed checksums") + } + if _, dup := out[name]; dup { + return nil, notSigned("duplicate checksum for " + name) + } + out[name] = [sha256.Size]byte(b) + } + return out, nil +} diff --git a/tools/release-sign/main.go b/tools/release-sign/main.go new file mode 100644 index 0000000..b77c8d1 --- /dev/null +++ b/tools/release-sign/main.go @@ -0,0 +1,169 @@ +// Command release-sign makes and uses the key that signs Syncer releases. +// +// release-sign -keygen -out new key; prints the public key +// release-sign -sign -tag -key-env writes .sig +// release-sign -verify -tag -pub checks against .sig +// release-sign -verify -tag -pubfile same, with the key Syncer embeds +// +// The signature covers the release tag as well as the file, so a release's +// files can't be passed off under another tag. +// +// The private key is the base64 of the 32-byte ed25519 seed (one line), the +// public key and signatures are base64 too. The private key is only ever +// read from a file this tool wrote or from an environment variable, and is +// never printed. +package main + +import ( + "crypto/ed25519" + "crypto/rand" + "encoding/base64" + "errors" + "flag" + "fmt" + "io" + "os" + "regexp" + "strings" + + "github.com/ApolloF/syncer/internal/update" +) + +func main() { + os.Exit(run(os.Args[1:], os.Getenv, os.Stdout, os.Stderr)) +} + +func run(args []string, getenv func(string) string, stdout, stderr io.Writer) int { + fs := flag.NewFlagSet("release-sign", flag.ContinueOnError) + fs.SetOutput(stderr) + keygen := fs.Bool("keygen", false, "make a new key pair") + out := fs.String("out", "", "with -keygen: file for the private key") + sign := fs.String("sign", "", "file to sign") + keyEnv := fs.String("key-env", "", "with -sign: environment variable holding the private key") + verify := fs.String("verify", "", "file to check against its .sig") + pub := fs.String("pub", "", "with -verify: public key") + pubFile := fs.String("pubfile", "", "with -verify: internal/update/pubkey.go, to use the key embedded there") + tag := fs.String("tag", "", "with -sign or -verify: the release tag") + if err := fs.Parse(args); err != nil { + return 2 + } + var err error + switch { + case *keygen && *sign == "" && *verify == "": + err = genKey(*out, stdout) + case *sign != "" && !*keygen && *verify == "": + err = signFile(*sign, *tag, getenv(*keyEnv), *keyEnv) + case *verify != "" && !*keygen && *sign == "": + err = verifyFile(*verify, *tag, *pub, *pubFile) + default: + fmt.Fprintln(stderr, "use exactly one of -keygen, -sign or -verify") + return 2 + } + if err != nil { + fmt.Fprintln(stderr, "release-sign:", err) + return 1 + } + return 0 +} + +// genKey writes a new private key to path (never over an existing file) and +// prints the matching public key. +func genKey(path string, stdout io.Writer) error { + if path == "" { + return errors.New("-keygen needs -out ") + } + pub, priv, err := ed25519.GenerateKey(rand.Reader) + if err != nil { + return err + } + f, err := os.OpenFile(path, os.O_WRONLY|os.O_CREATE|os.O_EXCL, 0o600) + if err != nil { + return err + } + _, err = fmt.Fprintln(f, base64.StdEncoding.EncodeToString(priv.Seed())) + if cerr := f.Close(); err == nil { + err = cerr + } + if err != nil { + _ = os.Remove(path) + return err + } + _, err = fmt.Fprintln(stdout, base64.StdEncoding.EncodeToString(pub)) + return err +} + +func parseKey(s string) (ed25519.PrivateKey, error) { + seed, err := base64.StdEncoding.DecodeString(strings.TrimSpace(s)) + if err != nil || len(seed) != ed25519.SeedSize { + return nil, errors.New("the private key isn't a base64 ed25519 seed") + } + return ed25519.NewKeyFromSeed(seed), nil +} + +func checkTag(tag string) error { + if tag == "" || strings.ContainsAny(tag, "\r\n") { + return errors.New("-tag needs the release tag, e.g. v1.2.0") + } + return nil +} + +func signFile(path, tag, key, keyEnv string) error { + if err := checkTag(tag); err != nil { + return err + } + if keyEnv == "" { + return errors.New("-sign needs -key-env ") + } + if strings.TrimSpace(key) == "" { + return fmt.Errorf("%s is empty", keyEnv) + } + priv, err := parseKey(key) + if err != nil { + return err + } + data, err := os.ReadFile(path) + if err != nil { + return err + } + sig := base64.StdEncoding.EncodeToString(ed25519.Sign(priv, update.SignedMessage(tag, data))) + return os.WriteFile(path+".sig", []byte(sig+"\n"), 0o644) +} + +var embeddedKeyLine = regexp.MustCompile(`(?m)^const releasePublicKey = "([^"]*)"\r?$`) + +func verifyFile(path, tag, pubB64, pubFile string) error { + if err := checkTag(tag); err != nil { + return err + } + if (pubB64 == "") == (pubFile == "") { + return errors.New("-verify needs one of -pub or -pubfile") + } + if pubFile != "" { + src, err := os.ReadFile(pubFile) + if err != nil { + return err + } + m := embeddedKeyLine.FindSubmatch(src) + if m == nil { + return fmt.Errorf("no releasePublicKey in %s", pubFile) + } + pubB64 = string(m[1]) + } + pub, err := base64.StdEncoding.DecodeString(strings.TrimSpace(pubB64)) + if err != nil || len(pub) != ed25519.PublicKeySize { + return errors.New("the public key isn't a base64 ed25519 public key (is the placeholder still in pubkey.go?)") + } + data, err := os.ReadFile(path) + if err != nil { + return err + } + raw, err := os.ReadFile(path + ".sig") + if err != nil { + return err + } + sig, err := base64.StdEncoding.DecodeString(strings.TrimSpace(string(raw))) + if err != nil || !ed25519.Verify(pub, update.SignedMessage(tag, data), sig) { + return errors.New("the signature doesn't match") + } + return nil +} diff --git a/tools/release-sign/main_test.go b/tools/release-sign/main_test.go new file mode 100644 index 0000000..d91e694 --- /dev/null +++ b/tools/release-sign/main_test.go @@ -0,0 +1,110 @@ +package main + +import ( + "bytes" + "os" + "path/filepath" + "strings" + "testing" +) + +func runTool(t *testing.T, env map[string]string, args ...string) (code int, stdout, stderr string) { + t.Helper() + var o, e bytes.Buffer + code = run(args, func(k string) string { return env[k] }, &o, &e) + return code, o.String(), e.String() +} + +func TestKeygenSignVerifyRoundtrip(t *testing.T) { + dir := t.TempDir() + keyFile := filepath.Join(dir, "key") + code, pub, errOut := runTool(t, nil, "-keygen", "-out", keyFile) + if code != 0 { + t.Fatalf("keygen: %d %s", code, errOut) + } + priv, err := os.ReadFile(keyFile) + if err != nil { + t.Fatal(err) + } + if strings.Contains(pub, strings.TrimSpace(string(priv))) || strings.Count(strings.TrimSpace(pub), "\n") != 0 { + t.Fatalf("keygen printed %q", pub) + } + pub = strings.TrimSpace(pub) + + sums := filepath.Join(dir, "SHA256SUMS.txt") + if err := os.WriteFile(sums, []byte("abc Syncer.exe\n"), 0o644); err != nil { + t.Fatal(err) + } + env := map[string]string{"KEY": string(priv)} + if code, out, errOut := runTool(t, env, "-sign", sums, "-tag", "v1.2.0", "-key-env", "KEY"); code != 0 || out != "" || strings.Contains(errOut, string(priv)) { + t.Fatalf("sign: %d %q %s", code, out, errOut) + } + if code, _, errOut := runTool(t, nil, "-verify", sums, "-tag", "v1.2.0", "-pub", pub); code != 0 { + t.Fatalf("verify: %d %s", code, errOut) + } + if code, _, _ := runTool(t, nil, "-verify", sums, "-tag", "v1.1.0", "-pub", pub); code == 0 { + t.Fatal("a signature for v1.2.0 verified as v1.1.0") + } + if code, _, _ := runTool(t, nil, "-verify", sums, "-pub", pub); code == 0 { + t.Fatal("verify without -tag succeeded") + } + + pubFile := filepath.Join(dir, "pubkey.go") + src := "package update\n\nconst releasePublicKey = \"" + pub + "\"\n" + if err := os.WriteFile(pubFile, []byte(src), 0o644); err != nil { + t.Fatal(err) + } + if code, _, errOut := runTool(t, nil, "-verify", sums, "-tag", "v1.2.0", "-pubfile", pubFile); code != 0 { + t.Fatalf("verify with -pubfile: %d %s", code, errOut) + } + // A Windows checkout has CRLF line endings. + if err := os.WriteFile(pubFile, []byte(strings.ReplaceAll(src, "\n", "\r\n")), 0o644); err != nil { + t.Fatal(err) + } + if code, _, errOut := runTool(t, nil, "-verify", sums, "-tag", "v1.2.0", "-pubfile", pubFile); code != 0 { + t.Fatalf("verify with a CRLF -pubfile: %d %s", code, errOut) + } + placeholder := "package update\n\nconst releasePublicKey = \"UNSET: paste the public key\"\n" + if err := os.WriteFile(pubFile, []byte(placeholder), 0o644); err != nil { + t.Fatal(err) + } + if code, _, _ := runTool(t, nil, "-verify", sums, "-tag", "v1.2.0", "-pubfile", pubFile); code == 0 { + t.Fatal("the placeholder key verified") + } + + if err := os.WriteFile(sums, []byte("abd Syncer.exe\n"), 0o644); err != nil { + t.Fatal(err) + } + if code, _, _ := runTool(t, nil, "-verify", sums, "-tag", "v1.2.0", "-pub", pub); code == 0 { + t.Fatal("a changed file verified") + } +} + +func TestKeygenRefusesToOverwrite(t *testing.T) { + keyFile := filepath.Join(t.TempDir(), "key") + if err := os.WriteFile(keyFile, []byte("keep"), 0o600); err != nil { + t.Fatal(err) + } + code, out, _ := runTool(t, nil, "-keygen", "-out", keyFile) + if code == 0 || out != "" { + t.Fatalf("keygen over an existing file: %d %q", code, out) + } + if b, _ := os.ReadFile(keyFile); string(b) != "keep" { + t.Fatalf("key file overwritten: %q", b) + } +} + +func TestSignNeedsAKeyInTheEnvironment(t *testing.T) { + f := filepath.Join(t.TempDir(), "SHA256SUMS.txt") + if err := os.WriteFile(f, []byte("x"), 0o644); err != nil { + t.Fatal(err) + } + for _, key := range []string{"", " ", "not base64!", "AAAA"} { + if code, _, _ := runTool(t, map[string]string{"KEY": key}, "-sign", f, "-tag", "v1.2.0", "-key-env", "KEY"); code == 0 { + t.Errorf("signed with key %q", key) + } + } + if _, err := os.Stat(f + ".sig"); !os.IsNotExist(err) { + t.Error("a signature was written without a valid key") + } +} From 5282c4b8d437548c21d28b25d49e5e42b0c1319d Mon Sep 17 00:00:00 2001 From: ApolloF <98666317+ApolloF@users.noreply.github.com> Date: Sun, 25 Oct 2026 21:34:24 +0100 Subject: [PATCH 2/2] Embed the release signing key --- internal/update/pubkey.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/update/pubkey.go b/internal/update/pubkey.go index 403296f..ab3d1cf 100644 --- a/internal/update/pubkey.go +++ b/internal/update/pubkey.go @@ -9,7 +9,7 @@ import ( // with (printed by `go run ./tools/release-sign -keygen -out `). While // it is not a valid key, no update verifies, so none is installed. CI checks // the signature it just made against this very line. -const releasePublicKey = "UNSET: paste the public key from release-sign -keygen" +const releasePublicKey = "FFmfut3fCN5x/yt9FOf4XePWu1K8UM/Z4pyCRLqben8=" // embeddedKey is releasePublicKey decoded; nil when that isn't a valid key. var embeddedKey = decodeKey(releasePublicKey)