krz/gitbay

store: seal secret columns at rest !495

merged merged by cmc on 2026-09-28 22:37 UTC · krz/gitbay:secrets-at-rest into main

30 files changed, +1970 −56

Layout: unified · split

.gitbay/wiki/Admin.org +42
@@ -18,8 +18,14 @@ install -m 755 gitbayd /usr/local/bin/
18adduser --system --group --home /var/lib/gitbay --shell /usr/sbin/nologin gitbay 18adduser --system --group --home /var/lib/gitbay --shell /usr/sbin/nologin gitbay
19install -d -o gitbay -g gitbay -m 750 /var/lib/gitbay 19install -d -o gitbay -g gitbay -m 750 /var/lib/gitbay
20gitbayd --config /etc/gitbay/config.toml check-config 20gitbayd --config /etc/gitbay/config.toml check-config
21gitbayd --config /etc/gitbay/config.toml admin secrets init
22chown gitbay:gitbay /etc/gitbay/secret.key
21#+end_src 23#+end_src
22 24
25=admin secrets init= refuses when the key file already exists, so a
26reinstall on the same host should skip it — =deploy/install.sh= does
27this with a file check before running it.
28
23=deploy/= in the source tree has a cloud-init file, a hardened systemd 29=deploy/= in the source tree has a cloud-init file, a hardened systemd
24unit, and a nightly backup timer. Run as the unprivileged =gitbay= user; 30unit, and a nightly backup timer. Run as the unprivileged =gitbay= user;
25the unit's =AmbientCapabilities=CAP_NET_BIND_SERVICE= covers ports 31the unit's =AmbientCapabilities=CAP_NET_BIND_SERVICE= covers ports
@@ -57,6 +63,10 @@ validation still prints, followed by the contradiction.
57 keys, ACME cache all live here. 63 keys, ACME cache all live here.
58- =site_url= (required) — canonical =https://host=; drives ACME, clone 64- =site_url= (required) — canonical =https://host=; drives ACME, clone
59 URLs, mail links. 65 URLs, mail links.
66- =secret_key_file= (default =/etc/gitbay/secret.key=) — the keys that
67 seal CI secrets, webhook secrets, mirror tokens and push device
68 tokens in the database. Must be outside =root=, mode 0600, readable
69 by the daemon's user. See "Secret key" below.
60- =source_repo= (optional, =owner/name=) — the repository this instance 70- =source_repo= (optional, =owner/name=) — the repository this instance
61 develops itself in. Startup warns when the running build's commit is 71 develops itself in. Startup warns when the running build's commit is
62 not on that repository's default branch, which is how a binary built 72 not on that repository's default branch, which is how a binary built
@@ -503,6 +513,38 @@ snapshots sit beside them and the data stays referenced. Snapshot IDs
503change; their times do not. Done for krz/keycask (formerly rust-pass) 513change; their times do not. Done for krz/keycask (formerly rust-pass)
504on 2026-09-18, across 22 snapshots. 514on 2026-09-18, across 22 snapshots.
505 515
516** Secret key
517
518CI secrets, webhook secrets, mirror tokens and APNs device tokens are
519stored sealed: AES-256-GCM under a key in =server.secret_key_file=,
520each value prefixed with the id of the key that sealed it
521(=gbs1:<id>:=). The key file is not in the database, not under
522=server.root=, and therefore in neither the local archives nor the
523main restic repository. It must be copied off the host separately;
524without it a restored database's secrets cannot be opened, and
525gitbayd refuses to start against them.
526
527#+begin_src sh
528gitbayd admin secrets init # once; deploy/install.sh does it on first install
529gitbayd admin secrets check # open every value, count by key
530gitbayd admin secrets rotate # new key, reseal, retire the old one (as root)
531#+end_src
532
533- Missing file: every gitbayd process that opens the database refuses
534 to run and names the path, including =serve= and, in system mode,
535 =authorized-keys=. =migrate= does not need it.
536- Wrong key: =serve= stops at startup naming the first row that does
537 not open; =secrets check= does the same without starting anything.
538- Upgrade: the first start after the upgrade seals every value still
539 in clear and logs =sealed secret values=.
540- Rotation: =rotate= adds a key, reseals every value under it in one
541 transaction, then removes the old keys. Run it as root, since it
542 replaces the key file in =/etc/gitbay=; the file keeps its owner.
543 The daemon re-reads the file when it changes, so it needs no
544 restart. Copy the new file off the host afterwards.
545- Push devices are looked up by the SHA-256 of their token
546 (=push_devices.token_hash=), since two seals of one token differ.
547
506* Upgrades 548* Upgrades
507 549
508Replace the binary, restart the unit. Migrations apply automatically and 550Replace the binary, restart the unit. Migrations apply automatically and
.gitbay/wiki/Architecture/03-Deployment.org +1
@@ -50,6 +50,7 @@ a database check.
50| =<root>/acme= | ACME account key and certificates | autocert defaults | 50| =<root>/acme= | ACME account key and certificates | autocert defaults |
51| =<root>/hooks= | generated hook scripts | 0755 | 51| =<root>/hooks= | generated hook scripts | 0755 |
52| =/etc/gitbay/config.toml= | configuration, including SMTP password | 0640 (cloud-init) | 52| =/etc/gitbay/config.toml= | configuration, including SMTP password | 0640 (cloud-init) |
53| =/etc/gitbay/secret.key= | keys sealing secret columns | 0600, owner =gitbay= (=deploy/install.sh=) |
53| =/var/backups/gitbay= | backup archives | 0750 (cloud-init) | 54| =/var/backups/gitbay= | backup archives | 0750 (cloud-init) |
54 55
55* Outbound connections from gitbayd 56* Outbound connections from gitbayd
.gitbay/wiki/Architecture/06-Data-and-Cryptography.org +10 −8
@@ -2,7 +2,7 @@
2 2
3* Data inventory 3* Data inventory
4 4
5Schema: =internal/store/migrations/=, 59 migrations. Classification: 5Schema: =internal/store/migrations/=, 66 migrations. Classification:
6*C* credential or secret, *P* personal data, *R* private repository 6*C* credential or secret, *P* personal data, *R* private repository
7content (as confidential as the repository), *O* operational. 7content (as confidential as the repository), *O* operational.
8 8
@@ -14,9 +14,9 @@ content (as confidential as the repository), *O* operational.
14| Collaboration | =issues=, =issue_*=, =merge_requests=, =mr_*=, =labels=, =milestones=, =mentions= | R | bodies of issues, comments and reviews | 14| Collaboration | =issues=, =issue_*=, =merge_requests=, =mr_*=, =labels=, =milestones=, =mentions= | R | bodies of issues, comments and reviews |
15| Releases, snippets | =releases=, =release_assets=, =snippets=, =snippet_files= | R | | 15| Releases, snippets | =releases=, =release_assets=, =snippets=, =snippet_files= | R | |
16| CI | =builds= (includes logs), =build_schedules=, =runner_repos=, =runner_seen= | R | build logs can echo anything a step prints | 16| CI | =builds= (includes logs), =build_schedules=, =runner_repos=, =runner_seen= | R | build logs can echo anything a step prints |
17| CI secrets | =build_secrets= | C | *plaintext* | 17| CI secrets | =build_secrets= | C | sealed (AES-256-GCM) |
18| Integrations | =webhooks= (secret), =webhook_deliveries=, =mirrors= (username, token) | C | *plaintext* secrets and tokens | 18| Integrations | =webhooks= (secret), =webhook_deliveries=, =mirrors= (username, token) | C | webhook secret and mirror token sealed |
19| Notifications | =notifications= (mail queue), =inbox=, =push_devices= (APNs token), =push_queue= | P | device tokens in clear | 19| Notifications | =notifications= (mail queue), =inbox=, =push_devices= (APNs token), =push_queue= | P | device tokens sealed; looked up by SHA-256 |
20| Signatures | =commit_signatures=, =settings.key_epoch= | O | verification cache | 20| Signatures | =commit_signatures=, =settings.key_epoch= | O | verification cache |
21| Audit and feed | =audit_log=, =events= | O, P | actor ids, pruned argv, fingerprints and IPs in some audit rows, a hash chain (=prev_hash=, =hash=) | 21| Audit and feed | =audit_log=, =events= | O, P | actor ids, pruned argv, fingerprints and IPs in some audit rows, a hash chain (=prev_hash=, =hash=) |
22| Dependencies | =dep_checks=, =dep_reports= | O | | 22| Dependencies | =dep_checks=, =dep_reports= | O | |
@@ -31,6 +31,7 @@ Outside the database:
31| TLS keys (ACME) | =<root>/acme= | C | 31| TLS keys (ACME) | =<root>/acme= | C |
32| SMTP password | =/etc/gitbay/config.toml= | C | 32| SMTP password | =/etc/gitbay/config.toml= | C |
33| APNs signing key (.p8) | path in =push.key_file= | C | 33| APNs signing key (.p8) | path in =push.key_file= | C |
34| Secret key file | =server.secret_key_file= (=/etc/gitbay/secret.key=) | C |
34| Backups | =/var/backups/gitbay=, offsite | all of the above | 35| Backups | =/var/backups/gitbay=, offsite | all of the above |
35 36
36No table stores client IP addresses as a column. The daemon writes a 37No table stores client IP addresses as a column. The daemon writes a
@@ -42,14 +43,15 @@ throttling (=internal/sshd/sshd.go=).
42| Item | Protection | 43| Item | Protection |
43|---------------------------------------+----------------------------------------------------------------| 44|---------------------------------------+----------------------------------------------------------------|
44| API tokens, sessions, login links, email codes, invites | SHA-256 of a 256-bit random value; the value is shown once and never stored (=internal/store/sessions.go=) | 45| API tokens, sessions, login links, email codes, invites | SHA-256 of a 256-bit random value; the value is shown once and never stored (=internal/store/sessions.go=) |
45| CI secrets, webhook secrets, mirror tokens, APNs device tokens | stored in clear in SQLite; protection is filesystem permissions and the rule that values are write-only through the interface | 46| CI secrets, webhook secrets, mirror tokens, APNs device tokens | AES-256-GCM under a key file outside the database and outside =server.root=; additional data binds table, column and row (=internal/seal=, =internal/store/secrets.go=) |
46| SQLite file | mode 0640, directory 0750 | 47| SQLite file | mode 0640, directory 0750 |
47| Backups | the local archive is not encrypted; restic encrypts the offsite copy | 48| Backups | the local archive is not encrypted; restic encrypts the offsite copy |
48| Disk | no application-level encryption; any disk encryption is the host's | 49| Disk | no application-level encryption; any disk encryption is the host's |
49 50
50The code base contains no symmetric encryption. A database or backup 51The database file or a backup read by anyone other than the =gitbay=
51file read by anyone other than the =gitbay= user discloses every CI 52user discloses no CI secret, webhook secret, mirror token or device
52secret, webhook secret and mirror token. 53token without the key file, which neither carries. Rotation:
54=gitbayd admin secrets rotate= (Admin wiki).
53 55
54* In transit 56* In transit
55 57
.gitbay/wiki/Architecture/09-Controls.org +1 −1
@@ -56,7 +56,7 @@ chapter names of OWASP ASVS 4.0 where one fits.
56| Control | Status | Evidence | 56| Control | Status | Evidence |
57|---------------------------------------------+----------+------------------------------------------------------------------| 57|---------------------------------------------+----------+------------------------------------------------------------------|
58| TLS for all authenticated HTTP | in place | ACME or certificate files; HSTS | 58| TLS for all authenticated HTTP | in place | ACME or certificate files; HSTS |
59| Secrets encrypted at rest | gap | CI secrets, webhook secrets, mirror tokens stored in clear (#273) | 59| Secrets encrypted at rest | in place | AES-256-GCM, key file outside the database and the main backups (=internal/seal=) |
60| Secrets kept out of argv, logs and output | in place | =ReadsStdin=, pruned audit argv, write-only secret commands | 60| Secrets kept out of argv, logs and output | in place | =ReadsStdin=, pruned audit argv, write-only secret commands |
61| Local backups encrypted | gap | tar.gz in clear; offsite copy encrypted by restic (#274) | 61| Local backups encrypted | gap | tar.gz in clear; offsite copy encrypted by restic (#274) |
62| Data retention configurable | in place | =[retention]= (=internal/config/config.go=) | 62| Data retention configurable | in place | =[retention]= (=internal/config/config.go=) |
.gitbay/wiki/Architecture/10-Known-Gaps.org −1
@@ -14,7 +14,6 @@ what the 2026-09-27 review found; remove a row when its issue closes.
14| #260 | CI network | Builds share the runner's source address; no egress policy | medium | 14| #260 | CI network | Builds share the runner's source address; no egress policy | medium |
15| #261 | Various | Migration foreign-key check after commit; three web writes bypass dispatch; documentation drift | medium | 15| #261 | Various | Migration foreign-key check after commit; three web writes bypass dispatch; documentation drift | medium |
16| #262 | Availability | No limit on concurrent git pack generation | high | 16| #262 | Availability | No limit on concurrent git pack generation | high |
17| #273 | Data at rest | CI secrets, webhook secrets and mirror tokens are stored in clear in SQLite | high |
18| #274 | Backups | The local backup archive is not encrypted | medium | 17| #274 | Backups | The local backup archive is not encrypted | medium |
19| #298 | SSRF | =repo import --from= fetches without an address check | medium | 18| #298 | SSRF | =repo import --from= fetches without an address check | medium |
20| #297 | Credentials | A browser session can mint tokens and keys that outlive it | low | 19| #297 | Credentials | A browser session can mint tokens and keys that outlive it | low |
CHANGELOG.org +13
@@ -123,6 +123,19 @@ for the eighteen commands whose CLI path differs from the registry's
123 browser (#269). 123 browser (#269).
124- Untrusted builds (merge requests from forks) get a fresh HOME removed after the build and no secrets; trusted builds keep a per-repository home under =<workdir>/trusted-home=. Deploy gitbayd before the runner; the old shared homes under the runner's workdir can be deleted. (#255) 124- Untrusted builds (merge requests from forks) get a fresh HOME removed after the build and no secrets; trusted builds keep a per-repository home under =<workdir>/trusted-home=. Deploy gitbayd before the runner; the old shared homes under the runner's workdir can be deleted. (#255)
125- =status set= refuses =ci/= contexts, which belong to the instance's builds. Build results are reused only from trusted builds on the same image. =repo settings require-contexts= names status contexts that must report green; setting any turns require-checks on, and one not yet reported counts as pending. (#258) 125- =status set= refuses =ci/= contexts, which belong to the instance's builds. Build results are reused only from trusted builds on the same image. =repo settings require-contexts= names status contexts that must report green; setting any turns require-checks on, and one not yet reported counts as pending. (#258)
126*Upgrade note.* gitbayd needs =server.secret_key_file= (default
127=/etc/gitbay/secret.key=) and refuses to start without it. Before
128replacing the binary, run =gitbayd admin secrets init= as root and
129=chown gitbay:gitbay /etc/gitbay/secret.key= (=deploy/install.sh= does
130both when the file is missing). The first start seals the stored
131secrets. Back the key file up separately: =admin backup= archives do
132not carry it (see the Admin wiki, "Secret key"). Downgrading to an
133earlier release after values are sealed is not supported: an older
134gitbayd reads a sealed value's =gbs1:...= prefix as the literal
135secret.
136- CI secrets, webhook secrets, mirror tokens and push device tokens are
137 stored sealed with AES-256-GCM (#273). =gitbayd admin secrets
138 init|rotate|check=.
126- Untrusted builds (merge requests from forks) get a fresh HOME 139- Untrusted builds (merge requests from forks) get a fresh HOME
127 removed after the build and no secrets; trusted builds keep a 140 removed after the build and no secrets; trusted builds keep a
128 per-repository home under =<workdir>/trusted-home=. Deploy gitbayd 141 per-repository home under =<workdir>/trusted-home=. Deploy gitbayd
cmd/gitbayd/backup_test.go +2 −4
@@ -8,8 +8,6 @@ import (
8 "path/filepath" 8 "path/filepath"
9 "sort" 9 "sort"
10 "testing" 10 "testing"
11
12 "gitbay.org/gitbay/internal/config"
13) 11)
14 12
15// members lists the archive's entries by name. 13// members lists the archive's entries by name.
@@ -43,8 +41,8 @@ func members(t *testing.T, path string) []string {
43// --db-only is what makes an hourly schedule affordable, so it has to leave 41// --db-only is what makes an hourly schedule affordable, so it has to leave
44// the repositories out and still carry a restorable database. 42// the repositories out and still carry a restorable database.
45func TestBackupDBOnlyOmitsRepositories(t *testing.T) { 43func TestBackupDBOnlyOmitsRepositories(t *testing.T) {
46 root := t.TempDir() 44 cfg := testConfig(t)
47 cfg := config.Config{Server: config.Server{Root: root}} 45 root := cfg.Server.Root
48 s, err := openStore(cfg) 46 s, err := openStore(cfg)
49 if err != nil { 47 if err != nil {
50 t.Fatal(err) 48 t.Fatal(err)
cmd/gitbayd/main.go +24
@@ -4,8 +4,10 @@ package main
4 4
5import ( 5import (
6 "context" 6 "context"
7 "errors"
7 "fmt" 8 "fmt"
8 "io" 9 "io"
10 "io/fs"
9 "log/slog" 11 "log/slog"
10 "net" 12 "net"
11 "net/http" 13 "net/http"
@@ -31,6 +33,7 @@ import (
31 "gitbay.org/gitbay/internal/mirror" 33 "gitbay.org/gitbay/internal/mirror"
32 "gitbay.org/gitbay/internal/notify" 34 "gitbay.org/gitbay/internal/notify"
33 "gitbay.org/gitbay/internal/push" 35 "gitbay.org/gitbay/internal/push"
36 "gitbay.org/gitbay/internal/seal"
34 "gitbay.org/gitbay/internal/sshd" 37 "gitbay.org/gitbay/internal/sshd"
35 "gitbay.org/gitbay/internal/store" 38 "gitbay.org/gitbay/internal/store"
36 "gitbay.org/gitbay/internal/toolpath" 39 "gitbay.org/gitbay/internal/toolpath"
@@ -38,10 +41,21 @@ import (
38) 41)
39 42
40func openStore(cfg config.Config) (*store.Store, error) { 43func openStore(cfg config.Config) (*store.Store, error) {
44 // The key file seals the secret columns (#273). Without it the
45 // database's secrets cannot be read or written, so nothing that
46 // opens the database runs.
47 keys, err := seal.Load(cfg.Server.SecretKeyFile)
48 if errors.Is(err, fs.ErrNotExist) {
49 return nil, fmt.Errorf("secret key file %s does not exist: create it with gitbayd admin secrets init, or restore it from its off-host copy (backups do not carry it)", cfg.Server.SecretKeyFile)
50 }
51 if err != nil {
52 return nil, fmt.Errorf("secret key file: %w", err)
53 }
41 s, err := store.Open(filepath.Join(cfg.Server.Root, "gitbay.db")) 54 s, err := store.Open(filepath.Join(cfg.Server.Root, "gitbay.db"))
42 if err != nil { 55 if err != nil {
43 return nil, err 56 return nil, err
44 } 57 }
58 s.SetKeyring(keys)
45 // Say so when the schema moves. A restart migrates in silence otherwise, 59 // Say so when the schema moves. A restart migrates in silence otherwise,
46 // which makes an unexpected schema version hard to attribute to the deploy 60 // which makes an unexpected schema version hard to attribute to the deploy
47 // that caused it. 61 // that caused it.
@@ -144,6 +158,15 @@ func serveCmd() *cobra.Command {
144 // audit row outside the database the daemon can write. Rows 158 // audit row outside the database the daemon can write. Rows
145 // are logged at Info, which the default handler always emits. 159 // are logged at Info, which the default handler always emits.
146 st.AuditJournal = slog.Default() 160 st.AuditJournal = slog.Default()
161 // Values stored before sealing existed, or under a key a
162 // rotation retired, are sealed under the current key before
163 // anything reads them. A value the key file cannot open
164 // stops the start here rather than failing each delivery.
165 if n, err := st.ResealSecrets(); err != nil {
166 return fmt.Errorf("sealing secrets under %s: %w (gitbayd admin secrets check lists every value that does not open)", cfg.Server.SecretKeyFile, err)
167 } else if n > 0 {
168 slog.Info("sealed secret values", "count", n)
169 }
147 170
148 // Regenerate hook scripts so a moved binary self-heals, then 171 // Regenerate hook scripts so a moved binary self-heals, then
149 // start the hook policy socket. 172 // start the hook policy socket.
@@ -422,6 +445,7 @@ func adminCmd() *cobra.Command {
422 hostCmd("runners [--json]", "runner accounts: last poll, scope, the build each holds", "admin", "runners"), 445 hostCmd("runners [--json]", "runner accounts: last poll, scope, the build each holds", "admin", "runners"),
423 auditCmd, 446 auditCmd,
424 backupCmd(), 447 backupCmd(),
448 secretsCmd(),
425 gcCmd(), 449 gcCmd(),
426 adminMigrateCommitRefsCmd(), 450 adminMigrateCommitRefsCmd(),
427 adminMigrateProfileAboutCmd(), 451 adminMigrateProfileAboutCmd(),
cmd/gitbayd/main_test.go +1 −3
@@ -6,15 +6,13 @@ import (
6 "strconv" 6 "strconv"
7 "strings" 7 "strings"
8 "testing" 8 "testing"
9
10 "gitbay.org/gitbay/internal/config"
11) 9)
12 10
13// A restart that moves the schema says so. Migrations used to run in silence, 11// A restart that moves the schema says so. Migrations used to run in silence,
14// which left an unexpected user_version with nothing in the journal tying it to 12// which left an unexpected user_version with nothing in the journal tying it to
15// the deploy that applied it. 13// the deploy that applied it.
16func TestOpenStoreLogsSchemaMigration(t *testing.T) { 14func TestOpenStoreLogsSchemaMigration(t *testing.T) {
17 cfg := config.Config{Server: config.Server{Root: t.TempDir()}} 15 cfg := testConfig(t)
18 16
19 var buf bytes.Buffer 17 var buf bytes.Buffer
20 prev := slog.Default() 18 prev := slog.Default()
cmd/gitbayd/secrets.go added +196
@@ -0,0 +1,196 @@
1package main
2
3import (
4 "errors"
5 "fmt"
6 "io"
7 "io/fs"
8 "os"
9 "sort"
10 "strings"
11 "syscall"
12
13 "github.com/spf13/cobra"
14
15 "gitbay.org/gitbay/internal/config"
16 "gitbay.org/gitbay/internal/seal"
17)
18
19// secretsCmd manages the key file that seals CI secrets, webhook
20// secrets, mirror tokens and push device tokens in the database. No
21// subcommand prints key material, only key ids.
22func secretsCmd() *cobra.Command {
23 cmd := &cobra.Command{
24 Use: "secrets",
25 Short: "the key file that seals secrets stored in the database",
26 }
27 run := func(f func(config.Config, io.Writer) error) func(*cobra.Command, []string) error {
28 return func(cmd *cobra.Command, args []string) error {
29 cfg, err := config.Load(configPath)
30 if err != nil {
31 return err
32 }
33 return f(cfg, os.Stdout)
34 }
35 }
36 cmd.AddCommand(
37 &cobra.Command{
38 Use: "init",
39 Short: "create the key file (server.secret_key_file) with one new key",
40 Long: `Creates server.secret_key_file, mode 0600, holding one new key. Run as
41root, the file is given to the owner of server.root, the daemon's user.
42Refuses when the file exists.`,
43 RunE: run(initSecrets),
44 },
45 &cobra.Command{
46 Use: "rotate",
47 Short: "seal every secret under a new key and retire the old ones",
48 Long: `Adds a new key to the key file, reseals every value under it in one
49transaction, then removes the old keys from the file. A running daemon
50re-reads the file when it changes, so no restart is needed. Run as the
51user that can replace the key file (root, for /etc/gitbay); the file
52keeps its owner. Copy the new file off the host afterwards.`,
53 RunE: run(rotateSecrets),
54 },
55 &cobra.Command{
56 Use: "check",
57 Short: "open every stored secret and count them per column by key; exit 1 if any does not open",
58 RunE: run(checkSecrets),
59 },
60 )
61 return cmd
62}
63
64// initSecrets writes a new key file. Run as root, it hands the file to
65// the owner of server.root, since the daemon reads it as that user.
66func initSecrets(cfg config.Config, w io.Writer) error {
67 path := cfg.Server.SecretKeyFile
68 if _, err := os.Lstat(path); err == nil {
69 return fmt.Errorf("%s already exists; gitbayd admin secrets rotate replaces its key", path)
70 } else if !errors.Is(err, fs.ErrNotExist) {
71 return err
72 }
73 uid, gid := -1, -1
74 if os.Geteuid() == 0 {
75 fi, err := os.Stat(cfg.Server.Root)
76 if err != nil {
77 return fmt.Errorf("the key file is given to the owner of server.root: %w", err)
78 }
79 st, ok := fi.Sys().(*syscall.Stat_t)
80 if !ok {
81 return fmt.Errorf("cannot read the owner of %s", cfg.Server.Root)
82 }
83 uid, gid = int(st.Uid), int(st.Gid)
84 }
85 k, err := seal.NewKey()
86 if err != nil {
87 return err
88 }
89 if err := seal.WriteKeys(path, []seal.Key{k}); err != nil {
90 return err
91 }
92 if uid >= 0 {
93 if err := os.Chown(path, uid, gid); err != nil {
94 // A root-owned file left behind would make a re-run refuse.
95 os.Remove(path)
96 return fmt.Errorf("could not give %s to the owner of %s, so it was removed: %w", path, cfg.Server.Root, err)
97 }
98 }
99 fmt.Fprintf(w, "wrote %s (key %s). Copy it off this host: backups do not carry it, and a restored database's secrets do not open without it.\n", path, k.ID)
100 return nil
101}
102
103// rotateSecrets adds a key, reseals under it, then drops the old keys.
104// Each step leaves a file that opens every stored value: after the first
105// write the file holds old and new keys; the reseal is one transaction;
106// the last write happens only after the reseal committed and every value
107// is confirmed under the new key. Interrupted anywhere, running it again
108// finishes the job.
109func rotateSecrets(cfg config.Config, w io.Writer) error {
110 path := cfg.Server.SecretKeyFile
111 old, err := seal.ReadKeys(path)
112 if err != nil {
113 return err
114 }
115 next, err := seal.NewKey()
116 if err != nil {
117 return err
118 }
119 if err := seal.WriteKeys(path, append(old, next)); err != nil {
120 return err
121 }
122 st, err := openStore(cfg)
123 if err != nil {
124 return err
125 }
126 defer st.Close()
127 keep := fmt.Sprintf("the key file holds the old keys and %s; run rotate again", next.ID)
128 n, err := st.ResealSecrets()
129 if err != nil {
130 return fmt.Errorf("resealing: %w (%s)", err, keep)
131 }
132 // Guards against a value sealed outside the reseal transaction under
133 // an old key; no test reaches it, since that needs a hook between the
134 // two calls.
135 use, err := st.SecretKeyUse()
136 if err != nil {
137 return fmt.Errorf("checking the reseal: %w (%s)", err, keep)
138 }
139 for id, c := range use {
140 if id != next.ID {
141 return fmt.Errorf("%d values are not under %s after the reseal (%s)", c, next.ID, keep)
142 }
143 }
144 if err := seal.WriteKeys(path, []seal.Key{next}); err != nil {
145 return err
146 }
147 retired := make([]string, len(old))
148 for i, k := range old {
149 retired[i] = k.ID
150 }
151 fmt.Fprintf(w, "key %s: resealed %d values; retired %s. Copy %s off this host.\n", next.ID, n, strings.Join(retired, ", "), path)
152 return nil
153}
154
155// checkSecrets opens every stored secret and prints, per column, how
156// many values each key sealed and every value that does not open. Any
157// such value is an error.
158func checkSecrets(cfg config.Config, w io.Writer) error {
159 st, err := openStore(cfg)
160 if err != nil {
161 return err
162 }
163 defer st.Close()
164 report, err := st.SecretReport()
165 if err != nil {
166 return err
167 }
168 failed := 0
169 for _, u := range report {
170 ids := make([]string, 0, len(u.ByKey))
171 for id := range u.ByKey {
172 ids = append(ids, id)
173 }
174 sort.Strings(ids)
175 var parts []string
176 for _, id := range ids {
177 if id == "" {
178 parts = append(parts, fmt.Sprintf("clear %d (sealed when the daemon next starts)", u.ByKey[id]))
179 } else {
180 parts = append(parts, fmt.Sprintf("key %s %d", id, u.ByKey[id]))
181 }
182 }
183 if len(parts) == 0 {
184 parts = []string{"none"}
185 }
186 fmt.Fprintf(w, "%s: %s\n", u.Column, strings.Join(parts, ", "))
187 for _, f := range u.Failed {
188 fmt.Fprintf(w, "%s row %d: %s\n", u.Column, f.RowID, f.Err)
189 failed++
190 }
191 }
192 if failed > 0 {
193 return fmt.Errorf("%s does not open %d stored values", cfg.Server.SecretKeyFile, failed)
194 }
195 return nil
196}
cmd/gitbayd/secrets_test.go added +176
@@ -0,0 +1,176 @@
1package main
2
3import (
4 "bytes"
5 "encoding/base64"
6 "os"
7 "path/filepath"
8 "strings"
9 "testing"
10
11 "gitbay.org/gitbay/internal/config"
12 "gitbay.org/gitbay/internal/seal"
13 "gitbay.org/gitbay/internal/store"
14)
15
16func TestOpenStoreRefusesWithoutKeyFile(t *testing.T) {
17 cfg := testConfig(t)
18 cfg.Server.SecretKeyFile = filepath.Join(t.TempDir(), "absent.key")
19 _, err := openStore(cfg)
20 if err == nil || !strings.Contains(err.Error(), "gitbayd admin secrets init") || !strings.Contains(err.Error(), cfg.Server.SecretKeyFile) {
21 t.Fatalf("openStore without a key file: %v", err)
22 }
23}
24
25// storeWithSecret opens cfg's store and stores one build secret.
26func storeWithSecret(t *testing.T, cfg config.Config) (*store.Store, int64) {
27 t.Helper()
28 st, err := openStore(cfg)
29 if err != nil {
30 t.Fatal(err)
31 }
32 uid, err := st.CreateUser("alice", false)
33 if err != nil {
34 t.Fatal(err)
35 }
36 repoID, err := st.CreateRepo("user", uid, "app", "public")
37 if err != nil {
38 t.Fatal(err)
39 }
40 if err := st.SetBuildSecret(repoID, "TOKEN", "v1"); err != nil {
41 t.Fatal(err)
42 }
43 return st, repoID
44}
45
46func TestRotateSecrets(t *testing.T) {
47 cfg := testConfig(t)
48 before, err := seal.ReadKeys(cfg.Server.SecretKeyFile)
49 if err != nil {
50 t.Fatal(err)
51 }
52 st, repoID := storeWithSecret(t, cfg)
53 st.Close()
54
55 var out bytes.Buffer
56 if err := rotateSecrets(cfg, &out); err != nil {
57 t.Fatalf("rotate: %v", err)
58 }
59 after, err := seal.ReadKeys(cfg.Server.SecretKeyFile)
60 if err != nil {
61 t.Fatal(err)
62 }
63 if len(after) != 1 || after[0].ID == before[0].ID {
64 t.Fatalf("key file after rotation holds %v, before %v", after, before)
65 }
66 assertNoKeyMaterial(t, out.String(), append(before, after...))
67 st, err = openStore(cfg)
68 if err != nil {
69 t.Fatal(err)
70 }
71 defer st.Close()
72 use, err := st.SecretKeyUse()
73 if err != nil || use[after[0].ID] != 1 || len(use) != 1 {
74 t.Fatalf("SecretKeyUse after rotation = %v, %v", use, err)
75 }
76 if got, _ := st.BuildSecrets(repoID); got["TOKEN"] != "v1" {
77 t.Fatalf("value after rotation: %v", got)
78 }
79}
80
81// A reseal that fails leaves the old keys in the file, so every value
82// still opens.
83func TestRotateSecretsKeepsOldKeysWhenResealFails(t *testing.T) {
84 cfg := testConfig(t)
85 before, err := seal.ReadKeys(cfg.Server.SecretKeyFile)
86 if err != nil {
87 t.Fatal(err)
88 }
89 st, repoID := storeWithSecret(t, cfg)
90 // A second value sealed under a key the file does not hold.
91 if _, err := st.DB.Exec("INSERT INTO build_secrets (repo_id, name, value) VALUES (?, 'BAD', 'gbs1:deadbeef:AAAA')", repoID); err != nil {
92 t.Fatal(err)
93 }
94 st.Close()
95
96 if err := rotateSecrets(cfg, &bytes.Buffer{}); err == nil {
97 t.Fatal("rotate succeeded over a value that does not open")
98 }
99 after, err := seal.ReadKeys(cfg.Server.SecretKeyFile)
100 if err != nil {
101 t.Fatal(err)
102 }
103 if len(after) != 2 || after[0].ID != before[0].ID {
104 t.Fatalf("key file after a failed rotation holds %v", after)
105 }
106}
107
108func TestInitSecrets(t *testing.T) {
109 cfg := testConfig(t)
110 cfg.Server.SecretKeyFile = filepath.Join(t.TempDir(), "secret.key")
111 var out bytes.Buffer
112 if err := initSecrets(cfg, &out); err != nil {
113 t.Fatal(err)
114 }
115 fi, err := os.Stat(cfg.Server.SecretKeyFile)
116 if err != nil {
117 t.Fatal(err)
118 }
119 if fi.Mode().Perm() != 0o600 {
120 t.Fatalf("mode %04o", fi.Mode().Perm())
121 }
122 keys, err := seal.ReadKeys(cfg.Server.SecretKeyFile)
123 if err != nil || len(keys) != 1 {
124 t.Fatalf("keys %v, %v", keys, err)
125 }
126 if !strings.Contains(out.String(), keys[0].ID) {
127 t.Fatalf("output does not name the key id: %q", out.String())
128 }
129 assertNoKeyMaterial(t, out.String(), keys)
130 if err := initSecrets(cfg, &out); err == nil || !strings.Contains(err.Error(), "already exists") {
131 t.Fatalf("second init: %v", err)
132 }
133 if again, _ := seal.ReadKeys(cfg.Server.SecretKeyFile); again[0].ID != keys[0].ID {
134 t.Fatal("second init replaced the key")
135 }
136}
137
138func TestCheckSecrets(t *testing.T) {
139 cfg := testConfig(t)
140 keys, err := seal.ReadKeys(cfg.Server.SecretKeyFile)
141 if err != nil {
142 t.Fatal(err)
143 }
144 st, repoID := storeWithSecret(t, cfg)
145
146 var out bytes.Buffer
147 if err := checkSecrets(cfg, &out); err != nil {
148 t.Fatalf("check: %v\n%s", err, out.String())
149 }
150 if !strings.Contains(out.String(), "build_secrets.value: key "+keys[0].ID+" 1") {
151 t.Fatalf("check output:\n%s", out.String())
152 }
153 assertNoKeyMaterial(t, out.String(), keys)
154
155 if _, err := st.DB.Exec("INSERT INTO build_secrets (repo_id, name, value) VALUES (?, 'BAD', 'gbs1:deadbeef:AAAA')", repoID); err != nil {
156 t.Fatal(err)
157 }
158 st.Close()
159 out.Reset()
160 err = checkSecrets(cfg, &out)
161 if err == nil || !strings.Contains(err.Error(), "does not open 1 stored value") {
162 t.Fatalf("check over a value that does not open: %v", err)
163 }
164 if !strings.Contains(out.String(), "deadbeef") || !strings.Contains(out.String(), "build_secrets.value row") {
165 t.Fatalf("check output does not name the failing row:\n%s", out.String())
166 }
167}
168
169func assertNoKeyMaterial(t *testing.T, out string, keys []seal.Key) {
170 t.Helper()
171 for _, k := range keys {
172 if strings.Contains(out, base64.StdEncoding.EncodeToString(k.Secret)) {
173 t.Fatalf("output carries key %s's secret", k.ID)
174 }
175 }
176}
cmd/gitbayd/testconfig_test.go added +24
@@ -0,0 +1,24 @@
1package main
2
3import (
4 "path/filepath"
5 "testing"
6
7 "gitbay.org/gitbay/internal/config"
8 "gitbay.org/gitbay/internal/seal"
9)
10
11// testConfig is a config with a fresh root and a key file outside it,
12// the minimum openStore accepts.
13func testConfig(t *testing.T) config.Config {
14 t.Helper()
15 key := filepath.Join(t.TempDir(), "secret.key")
16 k, err := seal.NewKey()
17 if err != nil {
18 t.Fatal(err)
19 }
20 if err := seal.WriteKeys(key, []seal.Key{k}); err != nil {
21 t.Fatal(err)
22 }
23 return config.Config{Server: config.Server{Root: t.TempDir(), SecretKeyFile: key}}
24}
deploy/install.sh +6
@@ -14,6 +14,12 @@ ssh -p "$port" "root@$host" '
14 chmod 755 /usr/local/bin/gitbayd.new 14 chmod 755 /usr/local/bin/gitbayd.new
15 mv /usr/local/bin/gitbayd.new /usr/local/bin/gitbayd 15 mv /usr/local/bin/gitbayd.new /usr/local/bin/gitbayd
16 /usr/local/bin/gitbayd --config /etc/gitbay/config.toml check-config --no-host-checks 16 /usr/local/bin/gitbayd --config /etc/gitbay/config.toml check-config --no-host-checks
17 # The key that seals secrets in the database. Created on the first
18 # install, never replaced here; gitbayd refuses to start without it.
19 if [ ! -e /etc/gitbay/secret.key ]; then
20 /usr/local/bin/gitbayd --config /etc/gitbay/config.toml admin secrets init
21 chown gitbay:gitbay /etc/gitbay/secret.key
22 fi
17 systemctl restart gitbayd 23 systemctl restart gitbayd
18 sleep 1 24 sleep 1
19 systemctl --no-pager --lines=5 status gitbayd 25 systemctl --no-pager --lines=5 status gitbayd
e2e/acme_test.go +2 −1
@@ -29,6 +29,7 @@ func TestACMEServe(t *testing.T) {
29[server] 29[server]
30root = %q 30root = %q
31site_url = "https://gitbay.example" 31site_url = "https://gitbay.example"
32secret_key_file = %q
32[ssh] 33[ssh]
33port = %d 34port = %d
34[http] 35[http]
@@ -36,7 +37,7 @@ addr = "127.0.0.1:%d"
36tls = "acme" 37tls = "acme"
37acme_email = "noreply@gitbay.example" 38acme_email = "noreply@gitbay.example"
38acme_http_addr = "127.0.0.1:%d" 39acme_http_addr = "127.0.0.1:%d"
39`, inst.root, inst.port, httpsPort, acmeHTTPPort) 40`, inst.root, inst.keyFile, inst.port, httpsPort, acmeHTTPPort)
40 if err := os.WriteFile(inst.config, []byte(cfg), 0o600); err != nil { 41 if err := os.WriteFile(inst.config, []byte(cfg), 0o600); err != nil {
41 t.Fatal(err) 42 t.Fatal(err)
42 } 43 }
e2e/backup_test.go +41 −1
@@ -1,16 +1,23 @@
1package e2e 1package e2e
2 2
3import ( 3import (
4 "bytes"
4 "fmt" 5 "fmt"
5 "net" 6 "net"
6 "os" 7 "os"
7 "os/exec" 8 "os/exec"
8 "path/filepath" 9 "path/filepath"
10 "regexp"
9 "strings" 11 "strings"
10 "testing" 12 "testing"
11 "time" 13 "time"
12) 14)
13 15
16// secretsCheckOneSealed matches "admin secrets check" reporting the one
17// build secret set in TestAdminBackup as sealed under some key, e.g.
18// "build_secrets.value: key 98e412e4 1".
19var secretsCheckOneSealed = regexp.MustCompile(`(?m)build_secrets\.value: key \S+ 1$`)
20
14func TestAdminBackup(t *testing.T) { 21func TestAdminBackup(t *testing.T) {
15 t.Parallel() 22 t.Parallel()
16 inst := startInstance(t) 23 inst := startInstance(t)
@@ -35,6 +42,11 @@ func TestAdminBackup(t *testing.T) {
35 if _, _, code := inst.ssh(t, aliceKey, "", "issue", "create", "alice/keep", "--title", "'survives backup'"); code != 0 { 42 if _, _, code := inst.ssh(t, aliceKey, "", "issue", "create", "alice/keep", "--title", "'survives backup'"); code != 0 {
36 t.Fatal("issue create failed") 43 t.Fatal("issue create failed")
37 } 44 }
45 // A build secret, to show the archive carries it sealed and the key
46 // file not at all.
47 if _, errOut, code := inst.ssh(t, aliceKey, "hunter2-at-rest", "repo", "secret", "set", "alice/keep", "DEPLOY_TOKEN"); code != 0 {
48 t.Fatalf("secret set: %s", errOut)
49 }
38 50
39 // Back up while the daemon is running. 51 // Back up while the daemon is running.
40 archive := filepath.Join(t.TempDir(), "backup.tar.gz") 52 archive := filepath.Join(t.TempDir(), "backup.tar.gz")
@@ -64,6 +76,16 @@ func TestAdminBackup(t *testing.T) {
64 } 76 }
65 } 77 }
66 } 78 }
79 if strings.Contains(names, "secret.key") {
80 t.Fatalf("archive carries the key file:\n%s", names)
81 }
82 db, err := exec.Command("tar", "-xzOf", archive, "gitbay.db").Output()
83 if err != nil {
84 t.Fatal(err)
85 }
86 if bytes.Contains(db, []byte("hunter2-at-rest")) {
87 t.Fatal("the archived database carries the build secret in clear")
88 }
67 89
68 // Restore: extract into a fresh root and serve from it. 90 // Restore: extract into a fresh root and serve from it.
69 root2 := t.TempDir() 91 root2 := t.TempDir()
@@ -77,12 +99,13 @@ func TestAdminBackup(t *testing.T) {
77[server] 99[server]
78root = %q 100root = %q
79site_url = "https://gitbay.test" 101site_url = "https://gitbay.test"
102secret_key_file = %q
80[ssh] 103[ssh]
81port = %d 104port = %d
82[http] 105[http]
83addr = "127.0.0.1:%d" 106addr = "127.0.0.1:%d"
84tls = "off" 107tls = "off"
85`, root2, port2, httpPort2) 108`, root2, inst.keyFile, port2, httpPort2)
86 if err := os.WriteFile(config2, []byte(cfg), 0o600); err != nil { 109 if err := os.WriteFile(config2, []byte(cfg), 0o600); err != nil {
87 t.Fatal(err) 110 t.Fatal(err)
88 } 111 }
@@ -149,6 +172,23 @@ tls = "off"
149 if code != 0 || strings.TrimSpace(out2) != "alice" { 172 if code != 0 || strings.TrimSpace(out2) != "alice" {
150 t.Fatalf("whoami on restored instance: exit %d, %q, %s", code, out2, errOut) 173 t.Fatalf("whoami on restored instance: exit %d, %q, %s", code, out2, errOut)
151 } 174 }
175 // With the original key the restored secrets open; with another key
176 // they do not.
177 if out, err := exec.Command(inst.gitbayd, "--config", config2, "admin", "secrets", "check").CombinedOutput(); err != nil || !secretsCheckOneSealed.Match(out) {
178 t.Fatalf("secrets check on the restored instance: %v\n%s", err, out)
179 }
180 config3 := filepath.Join(root2, "config-wrong-key.toml")
181 wrong := strings.Replace(cfg, fmt.Sprintf("secret_key_file = %q", inst.keyFile),
182 fmt.Sprintf("secret_key_file = %q", filepath.Join(t.TempDir(), "other.key")), 1)
183 if err := os.WriteFile(config3, []byte(wrong), 0o600); err != nil {
184 t.Fatal(err)
185 }
186 if out, err := exec.Command(inst.gitbayd, "--config", config3, "admin", "secrets", "init").CombinedOutput(); err != nil {
187 t.Fatalf("init the wrong key: %v\n%s", err, out)
188 }
189 if out, err := exec.Command(inst.gitbayd, "--config", config3, "admin", "secrets", "check").CombinedOutput(); err == nil || !strings.Contains(string(out), "does not hold") {
190 t.Fatalf("secrets check with the wrong key: %v\n%s", err, out)
191 }
152 if out2, _, code = ssh2("repo", "log", "alice/keep"); code != 0 || !strings.Contains(out2, "keep me") { 192 if out2, _, code = ssh2("repo", "log", "alice/keep"); code != 0 || !strings.Contains(out2, "keep me") {
153 t.Fatalf("restored log: %d\n%s", code, out2) 193 t.Fatalf("restored log: %d\n%s", code, out2)
154 } 194 }
e2e/ssh_test.go +6 −1
@@ -24,6 +24,7 @@ type instance struct {
24 gitPort int 24 gitPort int
25 proc *exec.Cmd 25 proc *exec.Cmd
26 sshDir string // per-user client keys live here 26 sshDir string // per-user client keys live here
27 keyFile string // server.secret_key_file, outside root
27} 28}
28 29
29// nextPort hands out candidate ports. Seeded randomly so two test processes 30// nextPort hands out candidate ports. Seeded randomly so two test processes
@@ -89,11 +90,13 @@ func startInstanceWith(t *testing.T, extra string) *instance {
89 gitPort: ports[2], 90 gitPort: ports[2],
90 sshDir: t.TempDir(), 91 sshDir: t.TempDir(),
91 } 92 }
93 inst.keyFile = filepath.Join(t.TempDir(), "secret.key")
92 inst.config = filepath.Join(inst.root, "config.toml") 94 inst.config = filepath.Join(inst.root, "config.toml")
93 cfg := fmt.Sprintf(` 95 cfg := fmt.Sprintf(`
94[server] 96[server]
95root = %q 97root = %q
96site_url = "https://gitbay.test" 98site_url = "https://gitbay.test"
99secret_key_file = %q
97[ssh] 100[ssh]
98port = %d 101port = %d
99[http] 102[http]
@@ -102,12 +105,14 @@ tls = "off"
102[git_daemon] 105[git_daemon]
103enabled = true 106enabled = true
104port = %d 107port = %d
105`, inst.root, inst.port, inst.httpPort, inst.gitPort) 108`, inst.root, inst.keyFile, inst.port, inst.httpPort, inst.gitPort)
106 cfg += extra + "\n" 109 cfg += extra + "\n"
107 if err := os.WriteFile(inst.config, []byte(cfg), 0o600); err != nil { 110 if err := os.WriteFile(inst.config, []byte(cfg), 0o600); err != nil {
108 t.Fatal(err) 111 t.Fatal(err)
109 } 112 }
110 113
114 inst.admin(t, "admin", "secrets", "init")
115
111 inst.proc = exec.Command(inst.gitbayd, "--config", inst.config, "serve") 116 inst.proc = exec.Command(inst.gitbayd, "--config", inst.config, "serve")
112 inst.proc.Stderr = os.Stderr 117 inst.proc.Stderr = os.Stderr
113 if err := inst.proc.Start(); err != nil { 118 if err := inst.proc.Start(); err != nil {
e2e/system_test.go +2 −1
@@ -33,12 +33,13 @@ func TestSystemSSHMode(t *testing.T) {
33[server] 33[server]
34root = %q 34root = %q
35site_url = "https://gitbay.test" 35site_url = "https://gitbay.test"
36secret_key_file = %q
36[ssh] 37[ssh]
37mode = "system" 38mode = "system"
38[http] 39[http]
39addr = "127.0.0.1:%d" 40addr = "127.0.0.1:%d"
40tls = "off" 41tls = "off"
41`, inst.root, inst.httpPort) 42`, inst.root, inst.keyFile, inst.httpPort)
42 if err := os.WriteFile(inst.config, []byte(cfg), 0o600); err != nil { 43 if err := os.WriteFile(inst.config, []byte(cfg), 0o600); err != nil {
43 t.Fatal(err) 44 t.Fatal(err)
44 } 45 }
internal/config/config.go +54 −1
@@ -9,6 +9,7 @@ import (
9 "fmt" 9 "fmt"
10 "net" 10 "net"
11 "os" 11 "os"
12 "path/filepath"
12 "strconv" 13 "strconv"
13 "strings" 14 "strings"
14 "time" 15 "time"
@@ -54,6 +55,11 @@ type Server struct {
54 // not on that repository's default branch. Empty disables the check, which 55 // not on that repository's default branch. Empty disables the check, which
55 // is right for any instance that does not host its own source. 56 // is right for any instance that does not host its own source.
56 SourceRepo string `toml:"source_repo"` 57 SourceRepo string `toml:"source_repo"`
58
59 // SecretKeyFile holds the keys that seal the secret columns of the
60 // database (internal/seal). It lives outside Root, so neither a
61 // backup archive nor a snapshot of Root carries it.
62 SecretKeyFile string `toml:"secret_key_file"`
57} 63}
58 64
59type SSH struct { 65type SSH struct {
@@ -294,7 +300,7 @@ func LoadAPNSKey(path string) (*ecdsa.PrivateKey, error) {
294// Default returns the configuration used when a key is absent from the file. 300// Default returns the configuration used when a key is absent from the file.
295func Default() Config { 301func Default() Config {
296 return Config{ 302 return Config{
297 Server: Server{Root: "/var/lib/gitbay"}, 303 Server: Server{Root: "/var/lib/gitbay", SecretKeyFile: "/etc/gitbay/secret.key"},
298 SSH: SSH{Mode: "embedded", Port: 22}, 304 SSH: SSH{Mode: "embedded", Port: 22},
299 HTTP: HTTP{Addr: ":443", TLS: "acme", ACMEHTTPAddr: ":80"}, 305 HTTP: HTTP{Addr: ":443", TLS: "acme", ACMEHTTPAddr: ":80"},
300 Web: Web{Mode: "view_only"}, 306 Web: Web{Mode: "view_only"},
@@ -330,6 +336,47 @@ func Load(path string) (Config, error) {
330 return cfg, cfg.Validate() 336 return cfg, cfg.Validate()
331} 337}
332 338
339// within reports whether path is dir or below it. Both are compared as
340// cleaned absolute paths (a relative path resolves against the working
341// directory, same as every other path in this config), with symlinks
342// resolved where the path exists on disk, so a path that reaches into dir
343// through a symlink, or through "..", is still reported as inside.
344func within(dir, path string) bool {
345 dir, path = resolvePath(dir), resolvePath(path)
346 rel, err := filepath.Rel(dir, path)
347 return err == nil && rel != ".." && !strings.HasPrefix(rel, ".."+string(filepath.Separator))
348}
349
350// resolvePath returns path as a cleaned absolute path, resolving symlinks in
351// it. The secret key file commonly does not exist yet (it is created by
352// `gitbayd admin secrets init`), and on this platform /var itself is a
353// symlink, so a whole-path resolution is tried first and, failing that, each
354// ancestor directory in turn, walking up to the nearest one that exists and
355// reattaching the missing suffix — a symlinked ancestor still resolves even
356// though the leaf, or several levels above it, does not exist.
357func resolvePath(path string) string {
358 abs, err := filepath.Abs(path)
359 if err != nil {
360 return filepath.Clean(path)
361 }
362 dir := abs
363 var suffix []string
364 for {
365 if resolved, err := filepath.EvalSymlinks(dir); err == nil {
366 for i := len(suffix) - 1; i >= 0; i-- {
367 resolved = filepath.Join(resolved, suffix[i])
368 }
369 return resolved
370 }
371 parent := filepath.Dir(dir)
372 if parent == dir {
373 return abs
374 }
375 suffix = append(suffix, filepath.Base(dir))
376 dir = parent
377 }
378}
379
333func oneOf(field, val string, allowed ...string) error { 380func oneOf(field, val string, allowed ...string) error {
334 for _, a := range allowed { 381 for _, a := range allowed {
335 if val == a { 382 if val == a {
@@ -357,6 +404,12 @@ func (c Config) Validate() error {
357 if c.Server.SiteURL == "" { 404 if c.Server.SiteURL == "" {
358 errs = append(errs, errors.New("server.site_url is required")) 405 errs = append(errs, errors.New("server.site_url is required"))
359 } 406 }
407 switch {
408 case c.Server.SecretKeyFile == "":
409 errs = append(errs, errors.New("server.secret_key_file is required"))
410 case within(c.Server.Root, c.Server.SecretKeyFile):
411 errs = append(errs, fmt.Errorf("server.secret_key_file %q is inside server.root: backups of the root would carry the key beside the values it seals", c.Server.SecretKeyFile))
412 }
360 if err := oneOf("ssh.mode", c.SSH.Mode, "embedded", "system"); err != nil { 413 if err := oneOf("ssh.mode", c.SSH.Mode, "embedded", "system"); err != nil {
361 errs = append(errs, err) 414 errs = append(errs, err)
362 } 415 }
internal/config/config_test.go +91
@@ -275,3 +275,94 @@ func TestMailTLSRequired(t *testing.T) {
275 } 275 }
276 } 276 }
277} 277}
278
279func TestSecretKeyFile(t *testing.T) {
280 cfg, err := Load(writeConfig(t, minimal))
281 if err != nil {
282 t.Fatal(err)
283 }
284 if cfg.Server.SecretKeyFile != "/etc/gitbay/secret.key" {
285 t.Errorf("default secret_key_file = %q", cfg.Server.SecretKeyFile)
286 }
287 for body, want := range map[string]string{
288 minimal + "secret_key_file = \"/var/lib/gitbay/secret.key\"\n": "inside server.root",
289 minimal + "secret_key_file = \"/var/lib/gitbay\"\n": "inside server.root",
290 minimal + "secret_key_file = \"\"\n": "server.secret_key_file is required",
291 } {
292 if _, err := Load(writeConfig(t, body)); err == nil || !strings.Contains(err.Error(), want) {
293 t.Errorf("%q: got %v, want an error containing %q", body, err, want)
294 }
295 }
296 if _, err := Load(writeConfig(t, minimal+"secret_key_file = \"/var/lib/gitbay-keys/secret.key\"\n")); err != nil {
297 t.Errorf("a sibling directory of the root is outside it: %v", err)
298 }
299}
300
301// TestSecretKeyFileSymlinks exercises resolvePath's symlink resolution: a
302// key path or root reached through a symlink is still compared on its
303// resolved location, not its literal spelling.
304func TestSecretKeyFileSymlinks(t *testing.T) {
305 valid := func(root, keyFile string) Config {
306 cfg := Default()
307 cfg.Server.SiteURL = "https://gitbay.example"
308 cfg.Server.Root = root
309 cfg.Server.SecretKeyFile = keyFile
310 return cfg
311 }
312
313 t.Run("key path reaches into root through a symlink", func(t *testing.T) {
314 tmp := t.TempDir()
315 root := filepath.Join(tmp, "root")
316 if err := os.Mkdir(root, 0o700); err != nil {
317 t.Fatal(err)
318 }
319 link := filepath.Join(tmp, "link-into-root")
320 if err := os.Symlink(root, link); err != nil {
321 t.Fatal(err)
322 }
323 // The key file itself need not exist yet; only the symlinked
324 // directory component does.
325 keyFile := filepath.Join(link, "secret.key")
326 if err := valid(root, keyFile).Validate(); err == nil || !strings.Contains(err.Error(), "inside server.root") {
327 t.Errorf("got %v, want an error containing %q", err, "inside server.root")
328 }
329 })
330
331 t.Run("root itself is reached through a symlinked parent", func(t *testing.T) {
332 tmp := t.TempDir()
333 actualRoot := filepath.Join(tmp, "actual", "root")
334 if err := os.MkdirAll(actualRoot, 0o700); err != nil {
335 t.Fatal(err)
336 }
337 rootLink := filepath.Join(tmp, "root-link")
338 if err := os.Symlink(actualRoot, rootLink); err != nil {
339 t.Fatal(err)
340 }
341 // server.root is configured as the symlink; the key file is given
342 // by its real, unsymlinked path under the same directory.
343 keyFile := filepath.Join(actualRoot, "secret.key")
344 if err := valid(rootLink, keyFile).Validate(); err == nil || !strings.Contains(err.Error(), "inside server.root") {
345 t.Errorf("got %v, want an error containing %q", err, "inside server.root")
346 }
347 })
348
349 t.Run("symlink points outside root", func(t *testing.T) {
350 tmp := t.TempDir()
351 root := filepath.Join(tmp, "root")
352 outside := filepath.Join(tmp, "outside")
353 if err := os.Mkdir(root, 0o700); err != nil {
354 t.Fatal(err)
355 }
356 if err := os.Mkdir(outside, 0o700); err != nil {
357 t.Fatal(err)
358 }
359 escape := filepath.Join(root, "escape")
360 if err := os.Symlink(outside, escape); err != nil {
361 t.Fatal(err)
362 }
363 keyFile := filepath.Join(escape, "secret.key")
364 if err := valid(root, keyFile).Validate(); err != nil {
365 t.Errorf("a symlink leading outside server.root should be accepted: %v", err)
366 }
367 })
368}
internal/seal/seal.go added +310
@@ -0,0 +1,310 @@
1// Package seal encrypts the secret columns of the database with
2// AES-256-GCM under keys held in a file outside the database and outside
3// server.root, so neither a copy of the database nor a backup opens them
4// (#273). A key file that cannot be re-read after it changes fails
5// closed: Seal and Open return errors until the file is fixed.
6package seal
7
8import (
9 "bufio"
10 "bytes"
11 "crypto/aes"
12 "crypto/cipher"
13 "crypto/rand"
14 "encoding/base64"
15 "encoding/hex"
16 "errors"
17 "fmt"
18 "io"
19 "os"
20 "path/filepath"
21 "strings"
22 "sync"
23 "syscall"
24)
25
26// Prefix marks a sealed value: "gbs1:<key id>:<base64 nonce||ciphertext>".
27const Prefix = "gbs1:"
28
29// maxKeyFile is the largest key file ReadKeys accepts.
30const maxKeyFile = 1 << 20
31
32// Key is one line of the key file.
33type Key struct {
34 ID string // 8 lowercase hex characters
35 Secret []byte // 32 bytes
36}
37
38// NewKey returns a key with a random id and secret.
39func NewKey() (Key, error) {
40 id := make([]byte, 4)
41 secret := make([]byte, 32)
42 if _, err := rand.Read(id); err != nil {
43 return Key{}, err
44 }
45 if _, err := rand.Read(secret); err != nil {
46 return Key{}, err
47 }
48 return Key{ID: hex.EncodeToString(id), Secret: secret}, nil
49}
50
51// ReadKeys reads the key file. The last key seals; every key opens.
52func ReadKeys(path string) ([]Key, error) {
53 f, err := os.Open(path)
54 if err != nil {
55 return nil, err
56 }
57 defer f.Close()
58 fi, err := f.Stat()
59 if err != nil {
60 return nil, err
61 }
62 if !fi.Mode().IsRegular() {
63 return nil, fmt.Errorf("%s is not a regular file", path)
64 }
65 if perm := fi.Mode().Perm(); perm&0o077 != 0 {
66 return nil, fmt.Errorf("%s is mode %04o; it must be readable by its owner alone (0600)", path, perm)
67 }
68 data, err := io.ReadAll(io.LimitReader(f, maxKeyFile+1))
69 if err != nil {
70 return nil, err
71 }
72 if len(data) > maxKeyFile {
73 return nil, fmt.Errorf("%s is larger than %d bytes", path, maxKeyFile)
74 }
75 var keys []Key
76 seen := map[string]bool{}
77 sc := bufio.NewScanner(bytes.NewReader(data))
78 for n := 1; sc.Scan(); n++ {
79 line := strings.TrimSpace(sc.Text())
80 if line == "" || strings.HasPrefix(line, "#") {
81 continue
82 }
83 f := strings.Fields(line)
84 if len(f) != 2 || !validID(f[0]) {
85 return nil, fmt.Errorf("%s:%d: want \"<8 hex id> <base64 32-byte key>\"", path, n)
86 }
87 secret, err := base64.StdEncoding.DecodeString(f[1])
88 if err != nil || len(secret) != 32 {
89 return nil, fmt.Errorf("%s:%d: key is not 32 bytes of base64", path, n)
90 }
91 if seen[f[0]] {
92 return nil, fmt.Errorf("%s:%d: key id %s appears twice", path, n, f[0])
93 }
94 seen[f[0]] = true
95 keys = append(keys, Key{ID: f[0], Secret: secret})
96 }
97 if err := sc.Err(); err != nil {
98 return nil, err
99 }
100 if len(keys) == 0 {
101 return nil, fmt.Errorf("%s holds no keys", path)
102 }
103 return keys, nil
104}
105
106// WriteKeys replaces the key file: a temporary file in the same
107// directory, mode 0600, given the existing file's owner when there is
108// one (rotation runs as root; the daemon reads the file as its own
109// user), then renamed over it. Keys ReadKeys would refuse are refused
110// before anything is written.
111func WriteKeys(path string, keys []Key) error {
112 if len(keys) == 0 {
113 return errors.New("no keys to write")
114 }
115 seen := map[string]bool{}
116 for _, k := range keys {
117 if !validID(k.ID) || len(k.Secret) != 32 || seen[k.ID] {
118 return fmt.Errorf("key %q is not an 8-hex-id, 32-byte key or appears twice", k.ID)
119 }
120 seen[k.ID] = true
121 }
122 var b strings.Builder
123 b.WriteString("# gitbay secret keys, \"<id> <base64 key>\" per line. The last line seals\n")
124 b.WriteString("# new values; the others open values sealed before a rotation.\n")
125 b.WriteString("# Keep a copy off this host: backups do not carry this file.\n")
126 for _, k := range keys {
127 fmt.Fprintf(&b, "%s %s\n", k.ID, base64.StdEncoding.EncodeToString(k.Secret))
128 }
129 dir := filepath.Dir(path)
130 tmp, err := os.CreateTemp(dir, ".secret-key-*")
131 if err != nil {
132 return err
133 }
134 defer os.Remove(tmp.Name())
135 fail := func(err error) error {
136 tmp.Close()
137 return err
138 }
139 if err := tmp.Chmod(0o600); err != nil {
140 return fail(err)
141 }
142 if fi, err := os.Stat(path); err == nil {
143 if st, ok := fi.Sys().(*syscall.Stat_t); ok {
144 if err := tmp.Chown(int(st.Uid), int(st.Gid)); err != nil {
145 return fail(err)
146 }
147 }
148 }
149 if _, err := tmp.WriteString(b.String()); err != nil {
150 return fail(err)
151 }
152 if err := tmp.Sync(); err != nil {
153 return fail(err)
154 }
155 if err := tmp.Close(); err != nil {
156 return err
157 }
158 if err := os.Rename(tmp.Name(), path); err != nil {
159 return err
160 }
161 // Losing the file loses every sealed value, so the rename is made
162 // durable before returning.
163 d, err := os.Open(dir)
164 if err == nil {
165 err = d.Sync()
166 d.Close()
167 }
168 if err != nil {
169 return fmt.Errorf("%s was replaced, but syncing %s failed: %w", path, dir, err)
170 }
171 return nil
172}
173
174// Keyring is the loaded key file. It re-reads the file whenever the file
175// changes, so a running daemon follows a rotation without a restart.
176type Keyring struct {
177 path string
178
179 mu sync.Mutex
180 fi os.FileInfo
181 cur string
182 aead map[string]cipher.AEAD
183}
184
185// Load reads the key file at path and returns a Keyring over it. It
186// fails when ReadKeys would.
187func Load(path string) (*Keyring, error) {
188 k := &Keyring{path: path}
189 if err := k.refresh(); err != nil {
190 return nil, err
191 }
192 return k, nil
193}
194
195// refresh reloads the file unless it is the one last read. Callers hold k.mu.
196func (k *Keyring) refresh() error {
197 fi, err := os.Stat(k.path)
198 if err != nil {
199 return err
200 }
201 if k.fi != nil && os.SameFile(k.fi, fi) && fi.ModTime().Equal(k.fi.ModTime()) && fi.Size() == k.fi.Size() {
202 return nil
203 }
204 keys, err := ReadKeys(k.path)
205 if err != nil {
206 return err
207 }
208 aead := make(map[string]cipher.AEAD, len(keys))
209 for _, key := range keys {
210 block, err := aes.NewCipher(key.Secret)
211 if err != nil {
212 return err
213 }
214 g, err := cipher.NewGCM(block)
215 if err != nil {
216 return err
217 }
218 aead[key.ID] = g
219 }
220 k.fi, k.cur, k.aead = fi, keys[len(keys)-1].ID, aead
221 return nil
222}
223
224// CurrentID is the id of the key that seals new values.
225func (k *Keyring) CurrentID() (string, error) {
226 k.mu.Lock()
227 defer k.mu.Unlock()
228 if err := k.refresh(); err != nil {
229 return "", err
230 }
231 return k.cur, nil
232}
233
234// Seal encrypts plain under the current key with a random nonce. aad
235// names the column, so a value copied into another column does not open
236// there.
237func (k *Keyring) Seal(aad, plain string) (string, error) {
238 if aad == "" {
239 return "", errNoAAD
240 }
241 k.mu.Lock()
242 defer k.mu.Unlock()
243 if err := k.refresh(); err != nil {
244 return "", err
245 }
246 g := k.aead[k.cur]
247 nonce := make([]byte, g.NonceSize())
248 if _, err := rand.Read(nonce); err != nil {
249 return "", err
250 }
251 ct := g.Seal(nonce, nonce, []byte(plain), []byte(aad))
252 return Prefix + k.cur + ":" + base64.RawStdEncoding.EncodeToString(ct), nil
253}
254
255// Open decrypts a value Seal produced under any key the file holds.
256func (k *Keyring) Open(aad, sealed string) (string, error) {
257 if aad == "" {
258 return "", errNoAAD
259 }
260 id, body, ok := split(sealed)
261 if !ok {
262 return "", errors.New("not a sealed value")
263 }
264 k.mu.Lock()
265 defer k.mu.Unlock()
266 if err := k.refresh(); err != nil {
267 return "", err
268 }
269 g, ok := k.aead[id]
270 if !ok {
271 return "", fmt.Errorf("sealed with key %s, which %s does not hold", id, k.path)
272 }
273 ct, err := base64.RawStdEncoding.DecodeString(body)
274 if err != nil || len(ct) < g.NonceSize()+g.Overhead() {
275 return "", fmt.Errorf("value sealed with key %s is malformed", id)
276 }
277 plain, err := g.Open(nil, ct[:g.NonceSize()], ct[g.NonceSize():], []byte(aad))
278 if err != nil {
279 return "", fmt.Errorf("value sealed with key %s does not open: wrong key, wrong column or altered value", id)
280 }
281 return string(plain), nil
282}
283
284var errNoAAD = errors.New("seal: additional data (table.column) is required")
285
286// IsSealed reports whether v carries the sealed prefix.
287func IsSealed(v string) bool { return strings.HasPrefix(v, Prefix) }
288
289// KeyID is the id of the key that sealed v.
290func KeyID(v string) (string, bool) {
291 id, _, ok := split(v)
292 return id, ok
293}
294
295func split(v string) (id, body string, ok bool) {
296 rest, ok := strings.CutPrefix(v, Prefix)
297 if !ok {
298 return "", "", false
299 }
300 id, body, ok = strings.Cut(rest, ":")
301 return id, body, ok && validID(id)
302}
303
304func validID(s string) bool {
305 if len(s) != 8 || strings.ToLower(s) != s {
306 return false
307 }
308 _, err := hex.DecodeString(s)
309 return err == nil
310}
internal/seal/seal_test.go added +234
@@ -0,0 +1,234 @@
1package seal
2
3import (
4 "encoding/base64"
5 "os"
6 "path/filepath"
7 "strings"
8 "testing"
9)
10
11func keyFile(t *testing.T, keys ...Key) string {
12 t.Helper()
13 path := filepath.Join(t.TempDir(), "secret.key")
14 if err := WriteKeys(path, keys); err != nil {
15 t.Fatal(err)
16 }
17 return path
18}
19
20func newKey(t *testing.T) Key {
21 t.Helper()
22 k, err := NewKey()
23 if err != nil {
24 t.Fatal(err)
25 }
26 return k
27}
28
29func TestSealOpenRoundTrip(t *testing.T) {
30 k := newKey(t)
31 ring, err := Load(keyFile(t, k))
32 if err != nil {
33 t.Fatal(err)
34 }
35 v, err := ring.Seal("build_secrets.value", "hunter2")
36 if err != nil {
37 t.Fatal(err)
38 }
39 if !strings.HasPrefix(v, Prefix+k.ID+":") || strings.Contains(v, "hunter2") {
40 t.Fatalf("sealed value %q", v)
41 }
42 if id, ok := KeyID(v); !ok || id != k.ID {
43 t.Fatalf("KeyID = %q, %v", id, ok)
44 }
45 got, err := ring.Open("build_secrets.value", v)
46 if err != nil || got != "hunter2" {
47 t.Fatalf("Open = %q, %v", got, err)
48 }
49 // Two seals of one value differ: the nonce is random.
50 if w, _ := ring.Seal("build_secrets.value", "hunter2"); w == v {
51 t.Fatal("two seals produced the same value")
52 }
53}
54
55// A value moved to another column does not open there.
56func TestOpenChecksAdditionalData(t *testing.T) {
57 ring, err := Load(keyFile(t, newKey(t)))
58 if err != nil {
59 t.Fatal(err)
60 }
61 v, _ := ring.Seal("mirrors.token", "tok")
62 if _, err := ring.Open("webhooks.secret", v); err == nil {
63 t.Fatal("opened under the wrong column")
64 }
65}
66
67func TestOpenRefusesAnAlteredValue(t *testing.T) {
68 ring, err := Load(keyFile(t, newKey(t)))
69 if err != nil {
70 t.Fatal(err)
71 }
72 v, _ := ring.Seal("mirrors.token", "tok")
73 // A character in the middle: the last one may carry only padding bits.
74 i := len(v) - 10
75 alt := byte('A')
76 if v[i] == 'A' {
77 alt = 'B'
78 }
79 if _, err := ring.Open("mirrors.token", v[:i]+string(alt)+v[i+1:]); err == nil {
80 t.Fatal("opened an altered value")
81 }
82 if _, err := ring.Open("mirrors.token", "tok"); err == nil {
83 t.Fatal("opened a clear value")
84 }
85}
86
87// A running daemon sees a rotation without a restart: the ring re-reads
88// the file when it changes.
89func TestKeyringFollowsTheFile(t *testing.T) {
90 old, next := newKey(t), newKey(t)
91 path := keyFile(t, old)
92 ring, err := Load(path)
93 if err != nil {
94 t.Fatal(err)
95 }
96 before, _ := ring.Seal("webhooks.secret", "s")
97 if err := WriteKeys(path, []Key{old, next}); err != nil {
98 t.Fatal(err)
99 }
100 after, err := ring.Seal("webhooks.secret", "s")
101 if err != nil {
102 t.Fatal(err)
103 }
104 if id, _ := KeyID(after); id != next.ID {
105 t.Fatalf("sealed under %s after rotation, want %s", id, next.ID)
106 }
107 if got, err := ring.Open("webhooks.secret", before); err != nil || got != "s" {
108 t.Fatalf("old value after rotation: %q, %v", got, err)
109 }
110 if err := WriteKeys(path, []Key{next}); err != nil {
111 t.Fatal(err)
112 }
113 if _, err := ring.Open("webhooks.secret", before); err == nil || !strings.Contains(err.Error(), old.ID) {
114 t.Fatalf("a retired key's value opened, or the error does not name the key: %v", err)
115 }
116}
117
118func TestReadKeysRefusesAReadableFile(t *testing.T) {
119 path := keyFile(t, newKey(t))
120 if err := os.Chmod(path, 0o640); err != nil {
121 t.Fatal(err)
122 }
123 if _, err := ReadKeys(path); err == nil || !strings.Contains(err.Error(), "0600") {
124 t.Fatalf("group-readable key file: %v", err)
125 }
126}
127
128func TestWriteKeysMode(t *testing.T) {
129 path := keyFile(t, newKey(t))
130 fi, err := os.Stat(path)
131 if err != nil {
132 t.Fatal(err)
133 }
134 if fi.Mode().Perm() != 0o600 {
135 t.Fatalf("mode %04o", fi.Mode().Perm())
136 }
137}
138
139func TestWriteKeysRefusesABadKey(t *testing.T) {
140 path := filepath.Join(t.TempDir(), "secret.key")
141 good := newKey(t)
142 for _, keys := range [][]Key{
143 nil,
144 {{ID: good.ID, Secret: good.Secret[:16]}},
145 {{ID: "XYZ12345", Secret: good.Secret}},
146 {good, good},
147 } {
148 if err := WriteKeys(path, keys); err == nil {
149 t.Errorf("wrote %d keys that do not read back", len(keys))
150 }
151 }
152 if _, err := os.Stat(path); !os.IsNotExist(err) {
153 t.Fatalf("a refused write left a file: %v", err)
154 }
155}
156
157func TestReadKeysRejectsMalformedLines(t *testing.T) {
158 for _, body := range []string{
159 "",
160 "# only a comment\n",
161 "XYZ12345 AAAA\n",
162 "0123abcd bm90IDMyIGJ5dGVz\n",
163 } {
164 path := filepath.Join(t.TempDir(), "k")
165 if err := os.WriteFile(path, []byte(body), 0o600); err != nil {
166 t.Fatal(err)
167 }
168 if _, err := ReadKeys(path); err == nil {
169 t.Errorf("accepted %q", body)
170 }
171 }
172}
173
174func TestOpenRefusesMalformedInput(t *testing.T) {
175 k := newKey(t)
176 ring, err := Load(keyFile(t, k))
177 if err != nil {
178 t.Fatal(err)
179 }
180 for _, v := range []string{
181 "gbs1:",
182 "gbs1:" + k.ID + ":",
183 "gbs1:" + strings.ToUpper(k.ID) + ":AAAA",
184 "gbs1:" + k.ID[:7] + ":AAAA",
185 "gbs1:" + k.ID + ":!!!not base64!!!",
186 "gbs1:" + k.ID + ":AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA", // 29 bytes
187 "gbs1:0badf00d:AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA",
188 } {
189 if got, err := ring.Open("mirrors.token", v); err == nil {
190 t.Errorf("Open(%q) = %q, want an error", v, got)
191 }
192 }
193}
194
195func TestEmptyAdditionalDataRefused(t *testing.T) {
196 ring, err := Load(keyFile(t, newKey(t)))
197 if err != nil {
198 t.Fatal(err)
199 }
200 if _, err := ring.Seal("", "s"); err == nil {
201 t.Fatal("sealed with no column")
202 }
203 v, _ := ring.Seal("mirrors.token", "s")
204 if _, err := ring.Open("", v); err == nil {
205 t.Fatal("opened with no column")
206 }
207}
208
209func TestReadKeysRefusesDuplicatesDirectoriesAndLargeFiles(t *testing.T) {
210 k := newKey(t)
211 line := k.ID + " " + base64.StdEncoding.EncodeToString(k.Secret) + "\n"
212 dup := filepath.Join(t.TempDir(), "dup")
213 if err := os.WriteFile(dup, []byte(line+line), 0o600); err != nil {
214 t.Fatal(err)
215 }
216 if _, err := ReadKeys(dup); err == nil || !strings.Contains(err.Error(), "twice") {
217 t.Errorf("duplicate id: %v", err)
218 }
219 dir := filepath.Join(t.TempDir(), "d")
220 if err := os.Mkdir(dir, 0o700); err != nil {
221 t.Fatal(err)
222 }
223 if _, err := ReadKeys(dir); err == nil {
224 t.Error("read a directory")
225 }
226 big := filepath.Join(t.TempDir(), "big")
227 body := line + "#" + strings.Repeat("x", maxKeyFile) + "\n"
228 if err := os.WriteFile(big, []byte(body), 0o600); err != nil {
229 t.Fatal(err)
230 }
231 if _, err := ReadKeys(big); err == nil {
232 t.Error("read a file over the size limit")
233 }
234}
internal/store/cisecrets.go +21 −5
@@ -1,13 +1,27 @@
1package store 1package store
2 2
3import "fmt"
4
3// SetBuildSecret stores or replaces one secret. The value never leaves the 5// SetBuildSecret stores or replaces one secret. The value never leaves the
4// server except inside a claimed build's environment. 6// server except inside a claimed build's environment. It is sealed inside
7// the write transaction; see ResealSecrets.
5func (s *Store) SetBuildSecret(repoID int64, name, value string) error { 8func (s *Store) SetBuildSecret(repoID int64, name, value string) error {
6 _, err := s.DB.Exec(` 9 tx, err := s.DB.Begin()
10 if err != nil {
11 return err
12 }
13 defer tx.Rollback()
14 sealed, err := s.sealValue(buildSecretAAD(repoID, name), value)
15 if err != nil {
16 return err
17 }
18 if _, err := tx.Exec(`
7 INSERT INTO build_secrets (repo_id, name, value) VALUES (?, ?, ?) 19 INSERT INTO build_secrets (repo_id, name, value) VALUES (?, ?, ?)
8 ON CONFLICT (repo_id, name) DO UPDATE SET value = excluded.value`, 20 ON CONFLICT (repo_id, name) DO UPDATE SET value = excluded.value`,
9 repoID, name, value) 21 repoID, name, sealed); err != nil {
10 return err 22 return err
23 }
24 return tx.Commit()
11} 25}
12 26
13func (s *Store) RemoveBuildSecret(repoID int64, name string) error { 27func (s *Store) RemoveBuildSecret(repoID int64, name string) error {
@@ -52,7 +66,9 @@ func (s *Store) BuildSecrets(repoID int64) (map[string]string, error) {
52 if err := rows.Scan(&n, &v); err != nil { 66 if err := rows.Scan(&n, &v); err != nil {
53 return nil, err 67 return nil, err
54 } 68 }
55 out[n] = v 69 if out[n], err = s.openValue(buildSecretAAD(repoID, n), v); err != nil {
70 return nil, fmt.Errorf("build secret %s: %w", n, err)
71 }
56 } 72 }
57 return out, rows.Err() 73 return out, rows.Err()
58} 74}
internal/store/migrations/0066_push_token_hash.down.sql added +2
@@ -0,0 +1,2 @@
1DROP INDEX push_devices_token_hash;
2ALTER TABLE push_devices DROP COLUMN token_hash;
internal/store/migrations/0066_push_token_hash.up.sql added +6
@@ -0,0 +1,6 @@
1-- APNs tokens are sealed with a random nonce (internal/seal), so two
2-- stores of one token differ; lookups and the re-registration upsert go
3-- by this SHA-256 of the token instead. Rows from before it are filled
4-- by Store.ResealSecrets.
5ALTER TABLE push_devices ADD COLUMN token_hash TEXT;
6CREATE UNIQUE INDEX push_devices_token_hash ON push_devices(token_hash);
internal/store/mirrors.go +39 −10
@@ -1,6 +1,9 @@
1package store 1package store
2 2
3import "errors" 3import (
4 "errors"
5 "fmt"
6)
4 7
5// ErrExists marks unique-constraint refusals callers turn into messages. 8// ErrExists marks unique-constraint refusals callers turn into messages.
6var ErrExists = errors.New("already exists") 9var ErrExists = errors.New("already exists")
@@ -20,28 +23,54 @@ type Mirror struct {
20 LastError string 23 LastError string
21} 24}
22 25
26// AddMirror stores the mirror, then seals its token under the new row's
27// id in the same transaction.
23func (s *Store) AddMirror(repoID int64, direction, url, username, token string) (int64, error) { 28func (s *Store) AddMirror(repoID int64, direction, url, username, token string) (int64, error) {
24 res, err := s.DB.Exec( 29 tx, err := s.DB.Begin()
25 "INSERT INTO mirrors (repo_id, direction, url, username, token) VALUES (?, ?, ?, ?, ?)", 30 if err != nil {
26 repoID, direction, url, username, token) 31 return 0, err
32 }
33 defer tx.Rollback()
34 res, err := tx.Exec(
35 "INSERT INTO mirrors (repo_id, direction, url, username, token) VALUES (?, ?, ?, ?, '')",
36 repoID, direction, url, username)
27 if err != nil { 37 if err != nil {
28 if isUniqueErr(err) { 38 if isUniqueErr(err) {
29 return 0, ErrExists 39 return 0, ErrExists
30 } 40 }
31 return 0, err 41 return 0, err
32 } 42 }
33 return res.LastInsertId() 43 id, err := res.LastInsertId()
44 if err != nil {
45 return 0, err
46 }
47 if token != "" {
48 sealed, err := s.sealValue(mirrorAAD(id), token)
49 if err != nil {
50 return 0, err
51 }
52 if _, err := tx.Exec("UPDATE mirrors SET token = ? WHERE id = ?", sealed, id); err != nil {
53 return 0, err
54 }
55 }
56 return id, tx.Commit()
34} 57}
35 58
36const mirrorSelect = ` 59const mirrorSelect = `
37 SELECT id, repo_id, direction, url, username, token, dirty, last_sync, last_error 60 SELECT id, repo_id, direction, url, username, token, dirty, last_sync, last_error
38 FROM mirrors` 61 FROM mirrors`
39 62
40func scanMirror(row interface{ Scan(...any) error }) (Mirror, error) { 63func (s *Store) scanMirror(row interface{ Scan(...any) error }) (Mirror, error) {
41 var m Mirror 64 var m Mirror
42 err := row.Scan(&m.ID, &m.RepoID, &m.Direction, &m.URL, &m.Username, &m.Token, 65 if err := row.Scan(&m.ID, &m.RepoID, &m.Direction, &m.URL, &m.Username, &m.Token,
43 &m.Dirty, &m.LastSync, &m.LastError) 66 &m.Dirty, &m.LastSync, &m.LastError); err != nil {
44 return m, err 67 return m, err
68 }
69 var err error
70 if m.Token, err = s.openValue(mirrorAAD(m.ID), m.Token); err != nil {
71 return m, fmt.Errorf("mirror %d: %w", m.ID, err)
72 }
73 return m, nil
45} 74}
46 75
47func (s *Store) mirrorQuery(q string, args ...any) ([]Mirror, error) { 76func (s *Store) mirrorQuery(q string, args ...any) ([]Mirror, error) {
@@ -52,7 +81,7 @@ func (s *Store) mirrorQuery(q string, args ...any) ([]Mirror, error) {
52 defer rows.Close() 81 defer rows.Close()
53 var out []Mirror 82 var out []Mirror
54 for rows.Next() { 83 for rows.Next() {
55 m, err := scanMirror(rows) 84 m, err := s.scanMirror(rows)
56 if err != nil { 85 if err != nil {
57 return nil, err 86 return nil, err
58 } 87 }
internal/store/push.go +37 −14
@@ -3,6 +3,7 @@ package store
3import ( 3import (
4 "database/sql" 4 "database/sql"
5 "errors" 5 "errors"
6 "fmt"
6 "time" 7 "time"
7) 8)
8 9
@@ -20,9 +21,11 @@ type PushDevice struct {
20 21
21// AddPushDevice registers a token to an account. A token already present 22// AddPushDevice registers a token to an account. A token already present
22// changes hands rather than erroring: Apple reuses tokens, and a reinstall 23// changes hands rather than erroring: Apple reuses tokens, and a reinstall
23// hands the same one to whichever account signs in next. The id is read 24// hands the same one to whichever account signs in next. The token is
24// back by token rather than taken from LastInsertId, which SQLite leaves 25// sealed (secrets.go), so the lookup and the upsert go by its hash, and a
25// unchanged when the DO UPDATE arm fires instead of the INSERT. 26// handover reseals it under the new owner. The id is read back by hash
27// rather than taken from LastInsertId, which SQLite leaves unchanged when
28// the DO UPDATE arm fires instead of the INSERT.
26// 29//
27// The row id survives that handover, so queue rows written for the 30// The row id survives that handover, so queue rows written for the
28// previous owner would still be delivered to the device — and an alert 31// previous owner would still be delivered to the device — and an alert
@@ -35,18 +38,27 @@ func (s *Store) AddPushDevice(userID int64, token, label string) (int64, error)
35 return 0, err 38 return 0, err
36 } 39 }
37 defer tx.Rollback() 40 defer tx.Rollback()
41 h := tokenHash(token)
42 // A row written before token_hash existed holds its token in clear.
43 if _, err := tx.Exec("UPDATE push_devices SET token_hash = ? WHERE token_hash IS NULL AND token = ?", h, token); err != nil {
44 return 0, err
45 }
38 var prev int64 46 var prev int64
39 if err := tx.QueryRow("SELECT user_id FROM push_devices WHERE token = ?", token).Scan(&prev); err != nil && !errors.Is(err, sql.ErrNoRows) { 47 if err := tx.QueryRow("SELECT user_id FROM push_devices WHERE token_hash = ?", h).Scan(&prev); err != nil && !errors.Is(err, sql.ErrNoRows) {
48 return 0, err
49 }
50 sealed, err := s.sealValue(pushTokenAAD(userID, h), token)
51 if err != nil {
40 return 0, err 52 return 0, err
41 } 53 }
42 if _, err := tx.Exec(` 54 if _, err := tx.Exec(`
43 INSERT INTO push_devices (user_id, token, label) VALUES (?, ?, ?) 55 INSERT INTO push_devices (user_id, token, token_hash, label) VALUES (?, ?, ?, ?)
44 ON CONFLICT(token) DO UPDATE SET user_id = excluded.user_id, label = excluded.label`, 56 ON CONFLICT(token_hash) DO UPDATE SET user_id = excluded.user_id, token = excluded.token, label = excluded.label`,
45 userID, token, label); err != nil { 57 userID, sealed, h, label); err != nil {
46 return 0, err 58 return 0, err
47 } 59 }
48 var id int64 60 var id int64
49 if err := tx.QueryRow("SELECT id FROM push_devices WHERE token = ?", token).Scan(&id); err != nil { 61 if err := tx.QueryRow("SELECT id FROM push_devices WHERE token_hash = ?", h).Scan(&id); err != nil {
50 return 0, err 62 return 0, err
51 } 63 }
52 if prev != 0 && prev != userID { 64 if prev != 0 && prev != userID {
@@ -60,7 +72,7 @@ func (s *Store) AddPushDevice(userID int64, token, label string) (int64, error)
60 72
61func (s *Store) PushDevices(userID int64) ([]PushDevice, error) { 73func (s *Store) PushDevices(userID int64) ([]PushDevice, error) {
62 rows, err := s.DB.Query(` 74 rows, err := s.DB.Query(`
63 SELECT id, user_id, token, label, created_at, COALESCE(last_seen_at, '') 75 SELECT id, user_id, token, COALESCE(token_hash, ''), label, created_at, COALESCE(last_seen_at, '')
64 FROM push_devices WHERE user_id = ? ORDER BY id`, userID) 76 FROM push_devices WHERE user_id = ? ORDER BY id`, userID)
65 if err != nil { 77 if err != nil {
66 return nil, err 78 return nil, err
@@ -69,9 +81,13 @@ func (s *Store) PushDevices(userID int64) ([]PushDevice, error) {
69 var out []PushDevice 81 var out []PushDevice
70 for rows.Next() { 82 for rows.Next() {
71 var d PushDevice 83 var d PushDevice
72 if err := rows.Scan(&d.ID, &d.UserID, &d.Token, &d.Label, &d.CreatedAt, &d.LastSeenAt); err != nil { 84 var h string
85 if err := rows.Scan(&d.ID, &d.UserID, &d.Token, &h, &d.Label, &d.CreatedAt, &d.LastSeenAt); err != nil {
73 return nil, err 86 return nil, err
74 } 87 }
88 if d.Token, err = s.openValue(pushTokenAAD(d.UserID, h), d.Token); err != nil {
89 return nil, fmt.Errorf("push device %d: %w", d.ID, err)
90 }
75 out = append(out, d) 91 out = append(out, d)
76 } 92 }
77 return out, rows.Err() 93 return out, rows.Err()
@@ -152,7 +168,7 @@ func (s *Store) EnqueuePush(userID int64, title, body, path string) error {
152 168
153func (s *Store) DuePush(limit int) ([]QueuedPush, error) { 169func (s *Store) DuePush(limit int) ([]QueuedPush, error) {
154 rows, err := s.DB.Query(` 170 rows, err := s.DB.Query(`
155 SELECT q.id, q.device_id, d.token, u.username, q.title, q.body, q.path, q.attempts, 171 SELECT q.id, q.device_id, d.token, d.user_id, COALESCE(d.token_hash, ''), u.username, q.title, q.body, q.path, q.attempts,
156 (SELECT COUNT(*) FROM inbox WHERE user_id = d.user_id AND read_at IS NULL) 172 (SELECT COUNT(*) FROM inbox WHERE user_id = d.user_id AND read_at IS NULL)
157 FROM push_queue q 173 FROM push_queue q
158 JOIN push_devices d ON d.id = q.device_id 174 JOIN push_devices d ON d.id = q.device_id
@@ -167,9 +183,14 @@ func (s *Store) DuePush(limit int) ([]QueuedPush, error) {
167 var out []QueuedPush 183 var out []QueuedPush
168 for rows.Next() { 184 for rows.Next() {
169 var p QueuedPush 185 var p QueuedPush
170 if err := rows.Scan(&p.ID, &p.DeviceID, &p.Token, &p.Username, &p.Title, &p.Body, &p.Path, &p.Attempts, &p.Badge); err != nil { 186 var uid int64
187 var h string
188 if err := rows.Scan(&p.ID, &p.DeviceID, &p.Token, &uid, &h, &p.Username, &p.Title, &p.Body, &p.Path, &p.Attempts, &p.Badge); err != nil {
171 return nil, err 189 return nil, err
172 } 190 }
191 if p.Token, err = s.openValue(pushTokenAAD(uid, h), p.Token); err != nil {
192 return nil, fmt.Errorf("push device %d: %w", p.DeviceID, err)
193 }
173 out = append(out, p) 194 out = append(out, p)
174 } 195 }
175 return out, rows.Err() 196 return out, rows.Err()
@@ -195,8 +216,10 @@ func (s *Store) MarkPushFailed(id int64, errMsg string, nextAt *time.Time) error
195} 216}
196 217
197// DeletePushDeviceByToken drops a device Apple has told us is gone. The 218// DeletePushDeviceByToken drops a device Apple has told us is gone. The
198// queue rows cascade, so nothing is left retrying at a dead token. 219// queue rows cascade, so nothing is left retrying at a dead token. A row
220// without a hash predates sealing and holds its token in clear.
199func (s *Store) DeletePushDeviceByToken(token string) error { 221func (s *Store) DeletePushDeviceByToken(token string) error {
200 _, err := s.DB.Exec("DELETE FROM push_devices WHERE token = ?", token) 222 _, err := s.DB.Exec("DELETE FROM push_devices WHERE token_hash = ? OR (token_hash IS NULL AND token = ?)",
223 tokenHash(token), token)
201 return err 224 return err
202} 225}
internal/store/secrets.go added +242
@@ -0,0 +1,242 @@
1package store
2
3import (
4 "crypto/sha256"
5 "database/sql"
6 "encoding/hex"
7 "errors"
8 "fmt"
9
10 "gitbay.org/gitbay/internal/seal"
11)
12
13// The additional data of a sealed value is "<table>.<column>:<row key>",
14// so a value copied into another column or another row does not open.
15// Each row key is known when the value is written and survives a
16// repository rename or transfer. Every read and write of a column builds
17// its additional data through the one function here.
18
19func buildSecretAAD(repoID int64, name string) string {
20 return fmt.Sprintf("build_secrets.value:%d/%s", repoID, name)
21}
22
23func webhookAAD(id int64) string { return fmt.Sprintf("webhooks.secret:%d", id) }
24
25func mirrorAAD(id int64) string { return fmt.Sprintf("mirrors.token:%d", id) }
26
27// pushTokenAAD names the owner as well as the token, so a handover to
28// another account reseals the token.
29func pushTokenAAD(userID int64, hash string) string {
30 return fmt.Sprintf("push_devices.token:%d/%s", userID, hash)
31}
32
33type secretColumn struct {
34 table, column string
35 // key selects the two parts of the row key, an integer and a text.
36 key string
37 aad func(n int64, s string) string
38}
39
40// secretColumns are the columns sealed under the key file (#273).
41var secretColumns = []secretColumn{
42 {"build_secrets", "value", "repo_id, name", buildSecretAAD},
43 {"webhooks", "secret", "id, ''", func(id int64, _ string) string { return webhookAAD(id) }},
44 {"mirrors", "token", "id, ''", func(id int64, _ string) string { return mirrorAAD(id) }},
45 {"push_devices", "token", "user_id, COALESCE(token_hash, '')", pushTokenAAD},
46}
47
48// SetKeyring sets the keys the secret columns are sealed under.
49func (s *Store) SetKeyring(k *seal.Keyring) { s.secrets = k }
50
51// sealValue seals v for storage. An empty value stays empty: for
52// webhooks and mirrors it means there is no secret.
53func (s *Store) sealValue(aad, v string) (string, error) {
54 if s.secrets == nil || v == "" {
55 return v, nil
56 }
57 return s.secrets.Seal(aad, v)
58}
59
60// openValue returns a stored value in clear. A value not yet sealed is
61// returned as stored: rows from before sealing existed stay readable
62// until ResealSecrets reaches them.
63func (s *Store) openValue(aad, v string) (string, error) {
64 if !seal.IsSealed(v) {
65 return v, nil
66 }
67 if s.secrets == nil {
68 return "", errors.New("value is sealed and no secret key is loaded")
69 }
70 return s.secrets.Open(aad, v)
71}
72
73// tokenHash is the lookup key for a push device token.
74func tokenHash(token string) string {
75 sum := sha256.Sum256([]byte(token))
76 return hex.EncodeToString(sum[:])
77}
78
79type secretRow struct {
80 rowid int64
81 value string
82 aad string
83}
84
85type queryer interface {
86 Query(query string, args ...any) (*sql.Rows, error)
87}
88
89func secretRows(q queryer, c secretColumn) ([]secretRow, error) {
90 rows, err := q.Query(fmt.Sprintf("SELECT rowid, %s, %s FROM %s WHERE %s != ''", c.column, c.key, c.table, c.column))
91 if err != nil {
92 return nil, err
93 }
94 defer rows.Close()
95 var out []secretRow
96 for rows.Next() {
97 var r secretRow
98 var n int64
99 var k string
100 if err := rows.Scan(&r.rowid, &r.value, &n, &k); err != nil {
101 return nil, err
102 }
103 r.aad = c.aad(n, k)
104 out = append(out, r)
105 }
106 return out, rows.Err()
107}
108
109// ResealSecrets fills push_devices.token_hash where it is missing, then
110// seals every clear value in the secret columns and reseals every value
111// not under the key file's current key. It runs in one write
112// transaction: every store write of a secret seals inside its own
113// transaction, so a write either lands before this one and is resealed,
114// or after it and is sealed under the key this one saw. It returns how
115// many values it rewrote.
116func (s *Store) ResealSecrets() (int, error) {
117 if s.secrets == nil {
118 return 0, errors.New("no secret key loaded")
119 }
120 tx, err := s.DB.Begin()
121 if err != nil {
122 return 0, err
123 }
124 defer tx.Rollback()
125 cur, err := s.secrets.CurrentID()
126 if err != nil {
127 return 0, err
128 }
129
130 // A token without a hash was written before sealing, so it is clear.
131 rows, err := tx.Query("SELECT id, token FROM push_devices WHERE token_hash IS NULL")
132 if err != nil {
133 return 0, err
134 }
135 var missing []secretRow
136 for rows.Next() {
137 var r secretRow
138 if err := rows.Scan(&r.rowid, &r.value); err != nil {
139 rows.Close()
140 return 0, err
141 }
142 missing = append(missing, r)
143 }
144 rows.Close()
145 if err := rows.Err(); err != nil {
146 return 0, err
147 }
148 for _, r := range missing {
149 if seal.IsSealed(r.value) {
150 return 0, fmt.Errorf("push_devices row %d: sealed token without a token_hash", r.rowid)
151 }
152 if _, err := tx.Exec("UPDATE push_devices SET token_hash = ? WHERE id = ?", tokenHash(r.value), r.rowid); err != nil {
153 return 0, err
154 }
155 }
156
157 n := 0
158 for _, c := range secretColumns {
159 rows, err := secretRows(tx, c)
160 if err != nil {
161 return 0, err
162 }
163 for _, r := range rows {
164 if id, ok := seal.KeyID(r.value); ok && id == cur {
165 continue
166 }
167 plain, err := s.openValue(r.aad, r.value)
168 if err != nil {
169 return 0, fmt.Errorf("%s.%s row %d: %w", c.table, c.column, r.rowid, err)
170 }
171 sealed, err := s.secrets.Seal(r.aad, plain)
172 if err != nil {
173 return 0, err
174 }
175 if _, err := tx.Exec(fmt.Sprintf("UPDATE %s SET %s = ? WHERE rowid = ?", c.table, c.column), sealed, r.rowid); err != nil {
176 return 0, err
177 }
178 n++
179 }
180 }
181 return n, tx.Commit()
182}
183
184// SecretColumnUse is one secret column's values by the id of the key
185// that sealed them ("" for a value still in clear), and the values that
186// do not open under the loaded key file.
187type SecretColumnUse struct {
188 Column string // "<table>.<column>"
189 ByKey map[string]int
190 Failed []SecretFailure
191}
192
193// SecretFailure is a stored value that does not open.
194type SecretFailure struct {
195 RowID int64
196 Err error
197}
198
199// SecretReport opens every value in the secret columns and counts them
200// per column by key id. A value that does not open is listed rather than
201// ending the scan.
202func (s *Store) SecretReport() ([]SecretColumnUse, error) {
203 var out []SecretColumnUse
204 for _, c := range secretColumns {
205 rows, err := secretRows(s.DB, c)
206 if err != nil {
207 return nil, err
208 }
209 u := SecretColumnUse{Column: c.table + "." + c.column, ByKey: map[string]int{}}
210 for _, r := range rows {
211 if _, err := s.openValue(r.aad, r.value); err != nil {
212 u.Failed = append(u.Failed, SecretFailure{RowID: r.rowid, Err: err})
213 continue
214 }
215 id, _ := seal.KeyID(r.value)
216 u.ByKey[id]++
217 }
218 out = append(out, u)
219 }
220 return out, nil
221}
222
223// SecretKeyUse counts the values in the secret columns by the id of the
224// key that sealed them ("" for a value still in clear), opening each
225// one, so a wrong or incomplete key file is an error naming the row.
226func (s *Store) SecretKeyUse() (map[string]int, error) {
227 report, err := s.SecretReport()
228 if err != nil {
229 return nil, err
230 }
231 use := map[string]int{}
232 for _, u := range report {
233 if len(u.Failed) > 0 {
234 f := u.Failed[0]
235 return nil, fmt.Errorf("%s row %d: %w", u.Column, f.RowID, f.Err)
236 }
237 for id, n := range u.ByKey {
238 use[id] += n
239 }
240 }
241 return use, nil
242}
internal/store/secrets_test.go added +349
@@ -0,0 +1,349 @@
1package store
2
3import (
4 "path/filepath"
5 "strings"
6 "testing"
7
8 "gitbay.org/gitbay/internal/seal"
9)
10
11// keyedStore is a migrated store with a key file of one key.
12func keyedStore(t *testing.T) (*Store, string, int64, int64) {
13 t.Helper()
14 s := open(t)
15 if err := s.MigrateUp(); err != nil {
16 t.Fatal(err)
17 }
18 path := filepath.Join(t.TempDir(), "secret.key")
19 k, err := seal.NewKey()
20 if err != nil {
21 t.Fatal(err)
22 }
23 if err := seal.WriteKeys(path, []seal.Key{k}); err != nil {
24 t.Fatal(err)
25 }
26 ring, err := seal.Load(path)
27 if err != nil {
28 t.Fatal(err)
29 }
30 s.SetKeyring(ring)
31 uid, err := s.CreateUser("alice", false)
32 if err != nil {
33 t.Fatal(err)
34 }
35 repoID, err := s.CreateRepo("user", uid, "app", "public")
36 if err != nil {
37 t.Fatal(err)
38 }
39 return s, path, uid, repoID
40}
41
42// raw reads every stored value of the secret columns.
43func raw(t *testing.T, s *Store) []string {
44 t.Helper()
45 var out []string
46 for _, sc := range secretColumns {
47 rows, err := s.DB.Query("SELECT " + sc.column + " FROM " + sc.table + " WHERE " + sc.column + " != ''")
48 if err != nil {
49 t.Fatal(err)
50 }
51 for rows.Next() {
52 var v string
53 if err := rows.Scan(&v); err != nil {
54 t.Fatal(err)
55 }
56 out = append(out, v)
57 }
58 rows.Close()
59 }
60 return out
61}
62
63func TestSecretColumnsAreSealed(t *testing.T) {
64 s, _, uid, repoID := keyedStore(t)
65 if err := s.SetBuildSecret(repoID, "DEPLOY", "ci-secret"); err != nil {
66 t.Fatal(err)
67 }
68 if _, err := s.AddWebhook(repoID, "https://hook.example/x", "hook-secret", "*"); err != nil {
69 t.Fatal(err)
70 }
71 if _, err := s.AddMirror(repoID, "push", "https://mirror.example/r.git", "u", "mirror-token"); err != nil {
72 t.Fatal(err)
73 }
74 if _, err := s.AddPushDevice(uid, "apns-token", "phone"); err != nil {
75 t.Fatal(err)
76 }
77
78 vals := raw(t, s)
79 if len(vals) != 4 {
80 t.Fatalf("stored %d values, want 4: %v", len(vals), vals)
81 }
82 for _, v := range vals {
83 if !seal.IsSealed(v) {
84 t.Errorf("stored in clear: %q", v)
85 }
86 for _, plain := range []string{"ci-secret", "hook-secret", "mirror-token", "apns-token"} {
87 if strings.Contains(v, plain) {
88 t.Errorf("%q carries %q", v, plain)
89 }
90 }
91 }
92
93 secrets, err := s.BuildSecrets(repoID)
94 if err != nil || secrets["DEPLOY"] != "ci-secret" {
95 t.Fatalf("BuildSecrets = %v, %v", secrets, err)
96 }
97 hooks, err := s.ListWebhooks(repoID)
98 if err != nil || len(hooks) != 1 || hooks[0].Secret != "hook-secret" {
99 t.Fatalf("ListWebhooks = %+v, %v", hooks, err)
100 }
101 ms, err := s.ListMirrors(repoID)
102 if err != nil || len(ms) != 1 || ms[0].Token != "mirror-token" {
103 t.Fatalf("ListMirrors = %+v, %v", ms, err)
104 }
105 due, err := s.DueMirrors(3600)
106 if err != nil || len(due) != 1 || due[0].Token != "mirror-token" {
107 t.Fatalf("DueMirrors = %+v, %v", due, err)
108 }
109 ds, err := s.PushDevices(uid)
110 if err != nil || len(ds) != 1 || ds[0].Token != "apns-token" {
111 t.Fatalf("PushDevices = %+v, %v", ds, err)
112 }
113
114 // An empty webhook secret or mirror token stays empty: it means none.
115 if _, err := s.AddWebhook(repoID, "https://hook.example/y", "", "*"); err != nil {
116 t.Fatal(err)
117 }
118 if _, err := s.AddMirror(repoID, "push", "https://mirror.example/s.git", "", ""); err != nil {
119 t.Fatal(err)
120 }
121 var empty int
122 s.DB.QueryRow("SELECT (SELECT COUNT(*) FROM webhooks WHERE secret = '') + (SELECT COUNT(*) FROM mirrors WHERE token = '')").Scan(&empty)
123 if empty != 2 {
124 t.Errorf("empty values stored as %d rows of '', want 2", empty)
125 }
126}
127
128// The queue readers open what they join.
129func TestSealedQueueReaders(t *testing.T) {
130 s, _, uid, repoID := keyedStore(t)
131 hook, err := s.AddWebhook(repoID, "https://hook.example/x", "hook-secret", "*")
132 if err != nil {
133 t.Fatal(err)
134 }
135 if _, err := s.DB.Exec("INSERT INTO events (repo_id, kind, data_json) VALUES (?, 'push', '{}')", repoID); err != nil {
136 t.Fatal(err)
137 }
138 if _, err := s.DB.Exec("INSERT INTO webhook_deliveries (webhook_id, event_id) SELECT ?, MAX(id) FROM events", hook); err != nil {
139 t.Fatal(err)
140 }
141 dd, err := s.DueDeliveries(10)
142 if err != nil || len(dd) != 1 || dd[0].Secret != "hook-secret" {
143 t.Fatalf("DueDeliveries = %+v, %v", dd, err)
144 }
145
146 if _, err := s.AddPushDevice(uid, "apns-token", "phone"); err != nil {
147 t.Fatal(err)
148 }
149 if err := s.EnqueuePush(uid, "t", "b", "/p"); err != nil {
150 t.Fatal(err)
151 }
152 qp, err := s.DuePush(10)
153 if err != nil || len(qp) != 1 || qp[0].Token != "apns-token" {
154 t.Fatalf("DuePush = %+v, %v", qp, err)
155 }
156}
157
158// A sealed value copied into another row of its column does not open:
159// the additional data names the row as well as the column.
160func TestSealedValueBoundToRow(t *testing.T) {
161 s, _, uid, repoID := keyedStore(t)
162 other, err := s.CreateRepo("user", uid, "other", "public")
163 if err != nil {
164 t.Fatal(err)
165 }
166 bob, err := s.CreateUser("bob", false)
167 if err != nil {
168 t.Fatal(err)
169 }
170 must := func(err error) {
171 t.Helper()
172 if err != nil {
173 t.Fatal(err)
174 }
175 }
176 must(s.SetBuildSecret(repoID, "A", "a"))
177 must(s.SetBuildSecret(repoID, "B", "b"))
178 must(s.SetBuildSecret(other, "A", "c"))
179 _, err = s.AddWebhook(repoID, "https://hook.example/1", "s1", "*")
180 must(err)
181 _, err = s.AddWebhook(repoID, "https://hook.example/2", "s2", "*")
182 must(err)
183 _, err = s.AddMirror(repoID, "push", "https://m.example/1.git", "", "t1")
184 must(err)
185 _, err = s.AddMirror(repoID, "pull", "https://m.example/2.git", "", "t2")
186 must(err)
187 _, err = s.AddPushDevice(uid, "d1", "")
188 must(err)
189 _, err = s.AddPushDevice(bob, "d2", "")
190 must(err)
191
192 // push_devices.token is unique, so alice's row goes before her
193 // sealed token is copied into bob's.
194 var aliceTok string
195 must(s.DB.QueryRow("SELECT token FROM push_devices WHERE user_id = ?", uid).Scan(&aliceTok))
196 _, err = s.DB.Exec("DELETE FROM push_devices WHERE user_id = ?", uid)
197 must(err)
198
199 cases := []struct {
200 name string
201 copy string
202 read func() error
203 }{
204 {"build secret to another name",
205 "UPDATE build_secrets SET value = (SELECT value FROM build_secrets WHERE repo_id = ?1 AND name = 'A') WHERE repo_id = ?1 AND name = 'B'",
206 func() error { _, err := s.BuildSecrets(repoID); return err }},
207 {"build secret to another repository",
208 "UPDATE build_secrets SET value = (SELECT value FROM build_secrets WHERE repo_id = ?1 AND name = 'A') WHERE repo_id = ?2 AND name = 'A'",
209 func() error { _, err := s.BuildSecrets(other); return err }},
210 {"webhook secret",
211 "UPDATE webhooks SET secret = (SELECT secret FROM webhooks WHERE url LIKE '%/1') WHERE url LIKE '%/2'",
212 func() error { _, err := s.ListWebhooks(repoID); return err }},
213 {"mirror token",
214 "UPDATE mirrors SET token = (SELECT token FROM mirrors WHERE direction = 'push') WHERE direction = 'pull'",
215 func() error { _, err := s.ListMirrors(repoID); return err }},
216 {"push token",
217 "UPDATE push_devices SET token = ?5 WHERE user_id = ?4",
218 func() error { _, err := s.PushDevices(bob); return err }},
219 }
220 for _, c := range cases {
221 if _, err := s.DB.Exec(c.copy, repoID, other, uid, bob, aliceTok); err != nil {
222 t.Fatalf("%s: %v", c.name, err)
223 }
224 if err := c.read(); err == nil {
225 t.Errorf("%s: a value copied from another row opened", c.name)
226 }
227 }
228}
229
230// A token re-registered under another account changes hands by its
231// hash, since two seals of one token differ.
232func TestPushDeviceUpsertBySealedToken(t *testing.T) {
233 s, _, uid, _ := keyedStore(t)
234 bob, err := s.CreateUser("bob", false)
235 if err != nil {
236 t.Fatal(err)
237 }
238 first, err := s.AddPushDevice(uid, "tok", "phone")
239 if err != nil {
240 t.Fatal(err)
241 }
242 second, err := s.AddPushDevice(bob, "tok", "ipad")
243 if err != nil {
244 t.Fatal(err)
245 }
246 if first != second {
247 t.Fatalf("re-registration made row %d beside %d", second, first)
248 }
249 d, err := s.PushDevices(bob)
250 if err != nil || len(d) != 1 || d[0].Token != "tok" {
251 t.Fatalf("PushDevices after handover = %+v, %v", d, err)
252 }
253 if err := s.DeletePushDeviceByToken("tok"); err != nil {
254 t.Fatal(err)
255 }
256 if d, _ := s.PushDevices(bob); len(d) != 0 {
257 t.Fatalf("device left after delete by token: %+v", d)
258 }
259}
260
261// A device row from before token_hash existed is found by its clear
262// token until ResealSecrets fills the hash.
263func TestPushDeviceUnhashedRow(t *testing.T) {
264 s, _, uid, _ := keyedStore(t)
265 if _, err := s.DB.Exec("INSERT INTO push_devices (user_id, token, label) VALUES (?, 'old', '')", uid); err != nil {
266 t.Fatal(err)
267 }
268 var first int64
269 s.DB.QueryRow("SELECT id FROM push_devices").Scan(&first)
270 id, err := s.AddPushDevice(uid, "old", "phone")
271 if err != nil || id != first {
272 t.Fatalf("AddPushDevice = %d, %v; want row %d", id, err, first)
273 }
274 if _, err := s.DB.Exec("INSERT INTO push_devices (user_id, token, label) VALUES (?, 'older', '')", uid); err != nil {
275 t.Fatal(err)
276 }
277 if err := s.DeletePushDeviceByToken("older"); err != nil {
278 t.Fatal(err)
279 }
280 if d, _ := s.PushDevices(uid); len(d) != 1 || d[0].Token != "old" {
281 t.Fatalf("PushDevices = %+v", d)
282 }
283}
284
285// Rows written before sealing existed, and rows under a retired key,
286// end up under the current key.
287func TestResealSecrets(t *testing.T) {
288 s, path, uid, repoID := keyedStore(t)
289 if _, err := s.DB.Exec("INSERT INTO build_secrets (repo_id, name, value) VALUES (?, 'OLD', 'clear-value')", repoID); err != nil {
290 t.Fatal(err)
291 }
292 if _, err := s.DB.Exec("INSERT INTO push_devices (user_id, token, label) VALUES (?, 'clear-token', '')", uid); err != nil {
293 t.Fatal(err)
294 }
295 n, err := s.ResealSecrets()
296 if err != nil || n != 2 {
297 t.Fatalf("ResealSecrets = %d, %v; want 2", n, err)
298 }
299 for _, v := range raw(t, s) {
300 if !seal.IsSealed(v) {
301 t.Errorf("still clear: %q", v)
302 }
303 }
304 var hash string
305 s.DB.QueryRow("SELECT COALESCE(token_hash, '') FROM push_devices").Scan(&hash)
306 if hash != tokenHash("clear-token") {
307 t.Errorf("token_hash = %q", hash)
308 }
309 if d, err := s.PushDevices(uid); err != nil || len(d) != 1 || d[0].Token != "clear-token" {
310 t.Fatalf("PushDevices after reseal = %+v, %v", d, err)
311 }
312 if n, _ := s.ResealSecrets(); n != 0 {
313 t.Errorf("second reseal rewrote %d values", n)
314 }
315
316 // Rotation: add a key, reseal, drop the old key; the value still opens.
317 old, err := seal.ReadKeys(path)
318 if err != nil {
319 t.Fatal(err)
320 }
321 next, _ := seal.NewKey()
322 if err := seal.WriteKeys(path, append(old, next)); err != nil {
323 t.Fatal(err)
324 }
325 if n, err := s.ResealSecrets(); err != nil || n != 2 {
326 t.Fatalf("reseal after rotation = %d, %v; want 2", n, err)
327 }
328 if err := seal.WriteKeys(path, []seal.Key{next}); err != nil {
329 t.Fatal(err)
330 }
331 use, err := s.SecretKeyUse()
332 if err != nil || use[next.ID] != 2 || len(use) != 1 {
333 t.Fatalf("SecretKeyUse = %v, %v", use, err)
334 }
335 if got, _ := s.BuildSecrets(repoID); got["OLD"] != "clear-value" {
336 t.Fatalf("value after rotation: %v", got)
337 }
338}
339
340func TestSealedValueWithoutKeyFails(t *testing.T) {
341 s, _, _, repoID := keyedStore(t)
342 if err := s.SetBuildSecret(repoID, "X", "v"); err != nil {
343 t.Fatal(err)
344 }
345 s.SetKeyring(nil)
346 if _, err := s.BuildSecrets(repoID); err == nil {
347 t.Fatal("opened a sealed value with no key loaded")
348 }
349}
internal/store/store.go +7 −1
@@ -15,6 +15,7 @@ import (
15 "strings" 15 "strings"
16 "sync" 16 "sync"
17 17
18 "gitbay.org/gitbay/internal/seal"
18 "modernc.org/sqlite" 19 "modernc.org/sqlite"
19) 20)
20 21
@@ -37,6 +38,9 @@ type Store struct {
37 // daemon sets it to its own logger, whose output the service 38 // daemon sets it to its own logger, whose output the service
38 // journal keeps outside the database. 39 // journal keeps outside the database.
39 AuditJournal *slog.Logger 40 AuditJournal *slog.Logger
41 // secrets seals and opens the secret columns (secrets.go). Nil
42 // stores values as given and refuses to open sealed ones.
43 secrets *seal.Keyring
40} 44}
41 45
42// Open opens (creating if needed) the database at path with WAL mode and 46// Open opens (creating if needed) the database at path with WAL mode and
@@ -54,7 +58,9 @@ type Store struct {
54// no failures, and readers, which WAL keeps out of the way, are 58// no failures, and readers, which WAL keeps out of the way, are
55// unaffected (#121). 59// unaffected (#121).
56func Open(path string) (*Store, error) { 60func Open(path string) (*Store, error) {
57 dsn := path + "?_txlock=immediate&_pragma=journal_mode(WAL)&_pragma=foreign_keys(ON)&_pragma=busy_timeout(5000)" 61 // synchronous(FULL): a commit is durable before it returns, which
62 // secret key rotation needs before it drops the old keys (#273).
63 dsn := path + "?_txlock=immediate&_pragma=journal_mode(WAL)&_pragma=synchronous(FULL)&_pragma=foreign_keys(ON)&_pragma=busy_timeout(5000)"
58 if path == ":memory:" { 64 if path == ":memory:" {
59 dsn = ":memory:?_txlock=immediate&_pragma=foreign_keys(ON)" 65 dsn = ":memory:?_txlock=immediate&_pragma=foreign_keys(ON)"
60 } 66 }
internal/store/webhooks.go +31 −4
@@ -1,6 +1,7 @@
1package store 1package store
2 2
3import ( 3import (
4 "fmt"
4 "time" 5 "time"
5) 6)
6 7
@@ -38,14 +39,34 @@ type DeliveryStatus struct {
38 CreatedAt string 39 CreatedAt string
39} 40}
40 41
42// AddWebhook stores the hook, then seals its secret under the new row's
43// id in the same transaction.
41func (s *Store) AddWebhook(repoID int64, url, secret, events string) (int64, error) { 44func (s *Store) AddWebhook(repoID int64, url, secret, events string) (int64, error) {
42 res, err := s.DB.Exec( 45 tx, err := s.DB.Begin()
43 "INSERT INTO webhooks (repo_id, url, secret, events) VALUES (?, ?, ?, ?)",
44 repoID, url, secret, events)
45 if err != nil { 46 if err != nil {
46 return 0, err 47 return 0, err
47 } 48 }
48 return res.LastInsertId() 49 defer tx.Rollback()
50 res, err := tx.Exec(
51 "INSERT INTO webhooks (repo_id, url, secret, events) VALUES (?, ?, '', ?)",
52 repoID, url, events)
53 if err != nil {
54 return 0, err
55 }
56 id, err := res.LastInsertId()
57 if err != nil {
58 return 0, err
59 }
60 if secret != "" {
61 sealed, err := s.sealValue(webhookAAD(id), secret)
62 if err != nil {
63 return 0, err
64 }
65 if _, err := tx.Exec("UPDATE webhooks SET secret = ? WHERE id = ?", sealed, id); err != nil {
66 return 0, err
67 }
68 }
69 return id, tx.Commit()
49} 70}
50 71
51func (s *Store) ListWebhooks(repoID int64) ([]Webhook, error) { 72func (s *Store) ListWebhooks(repoID int64) ([]Webhook, error) {
@@ -62,6 +83,9 @@ func (s *Store) ListWebhooks(repoID int64) ([]Webhook, error) {
62 if err := rows.Scan(&w.ID, &w.URL, &w.Secret, &w.Events, &active, &w.CreatedAt); err != nil { 83 if err := rows.Scan(&w.ID, &w.URL, &w.Secret, &w.Events, &active, &w.CreatedAt); err != nil {
63 return nil, err 84 return nil, err
64 } 85 }
86 if w.Secret, err = s.openValue(webhookAAD(w.ID), w.Secret); err != nil {
87 return nil, fmt.Errorf("webhook %d: %w", w.ID, err)
88 }
65 w.Active = active != 0 89 w.Active = active != 0
66 out = append(out, w) 90 out = append(out, w)
67 } 91 }
@@ -107,6 +131,9 @@ func (s *Store) DueDeliveries(limit int) ([]Delivery, error) {
107 &d.RepoPath, &d.Actor, &d.DataJSON, &d.EventAt, &d.Attempts); err != nil { 131 &d.RepoPath, &d.Actor, &d.DataJSON, &d.EventAt, &d.Attempts); err != nil {
108 return nil, err 132 return nil, err
109 } 133 }
134 if d.Secret, err = s.openValue(webhookAAD(d.WebhookID), d.Secret); err != nil {
135 return nil, fmt.Errorf("webhook %d: %w", d.WebhookID, err)
136 }
110 out = append(out, d) 137 out = append(out, d)
111 } 138 }
112 return out, rows.Err() 139 return out, rows.Err()