Commit 4cf9a72907
4cf9a72907023f28b9fae136175c05b4e948fd3c
parent: 72d22fe4f7
Verified · cmc
cmc <hello@cleberg.net> · 2026-09-28 22:30 UTC
secrets: serialize init and rotate on a lock beside the key file
Ref #273
Layout: unified · split
.gitbay/wiki/Admin.org
+3 −1
| @@ -605,7 +605,9 @@ gitbayd admin secrets rotate # new key, reseal, retire the old one (as root) |
| 605 | 605 | transaction, then removes the old keys. Run it as root, since it |
| 606 | 606 | replaces the key file in =/etc/gitbay=; the file keeps its owner. |
| 607 | 607 | The daemon re-reads the file when it changes, so it needs no |
| 608 | | restart. Copy the new file off the host afterwards. |
| 608 | restart. Copy the new file off the host afterwards. =init= and |
| 609 | =rotate= hold an flock on =<key file>.lock= while they run, so a |
| 610 | second run waits for the first. |
| 609 | 611 | - Push devices are looked up by the SHA-256 of their token |
| 610 | 612 | (=push_devices.token_hash=), since two seals of one token differ. |
| 611 | 613 | |
cmd/gitbayd/secrets.go
+27
| @@ -65,6 +65,11 @@ keeps its owner. Copy the new file off the host afterwards.`, |
| 65 | 65 | // the owner of server.root, since the daemon reads it as that user. |
| 66 | 66 | func initSecrets(cfg config.Config, w io.Writer) error { |
| 67 | 67 | path := cfg.Server.SecretKeyFile |
| 68 | unlock, err := lockKeyFile(path) |
| 69 | if err != nil { |
| 70 | return err |
| 71 | } |
| 72 | defer unlock() |
| 68 | 73 | if _, err := os.Lstat(path); err == nil { |
| 69 | 74 | return fmt.Errorf("%s already exists; gitbayd admin secrets rotate replaces its key", path) |
| 70 | 75 | } else if !errors.Is(err, fs.ErrNotExist) { |
| @@ -108,6 +113,11 @@ func initSecrets(cfg config.Config, w io.Writer) error { |
| 108 | 113 | // finishes the job. |
| 109 | 114 | func rotateSecrets(cfg config.Config, w io.Writer) error { |
| 110 | 115 | path := cfg.Server.SecretKeyFile |
| 116 | unlock, err := lockKeyFile(path) |
| 117 | if err != nil { |
| 118 | return err |
| 119 | } |
| 120 | defer unlock() |
| 111 | 121 | old, err := seal.ReadKeys(path) |
| 112 | 122 | if err != nil { |
| 113 | 123 | return err |
| @@ -152,6 +162,23 @@ func rotateSecrets(cfg config.Config, w io.Writer) error { |
| 152 | 162 | return nil |
| 153 | 163 | } |
| 154 | 164 | |
| 165 | // lockKeyFile takes an exclusive flock on <path>.lock, waiting for |
| 166 | // another init or rotate to finish. Two rotations interleaved would each |
| 167 | // write a file without the other's new key, and values resealed under |
| 168 | // the lost one would no longer open. O_NOFOLLOW refuses a symlink planted |
| 169 | // at the lock's name. |
| 170 | func lockKeyFile(path string) (func(), error) { |
| 171 | f, err := os.OpenFile(path+".lock", os.O_RDWR|os.O_CREATE|syscall.O_NOFOLLOW, 0o600) |
| 172 | if err != nil { |
| 173 | return nil, fmt.Errorf("key file lock: %w", err) |
| 174 | } |
| 175 | if err := syscall.Flock(int(f.Fd()), syscall.LOCK_EX); err != nil { |
| 176 | f.Close() |
| 177 | return nil, fmt.Errorf("key file lock: %w", err) |
| 178 | } |
| 179 | return func() { f.Close() }, nil |
| 180 | } |
| 181 | |
| 155 | 182 | // checkSecrets opens every stored secret and prints, per column, how |
| 156 | 183 | // many values each key sealed and every value that does not open. Any |
| 157 | 184 | // such value is an error. |
cmd/gitbayd/secrets_test.go
+48
| @@ -7,6 +7,7 @@ import ( |
| 7 | 7 | "path/filepath" |
| 8 | 8 | "strings" |
| 9 | 9 | "testing" |
| 10 | "time" |
| 10 | 11 | |
| 11 | 12 | "gitbay.org/gitbay/internal/config" |
| 12 | 13 | "gitbay.org/gitbay/internal/seal" |
| @@ -174,3 +175,50 @@ func assertNoKeyMaterial(t *testing.T, out string, keys []seal.Key) { |
| 174 | 175 | } |
| 175 | 176 | } |
| 176 | 177 | } |
| 178 | |
| 179 | // A rotate waits while another holds the key file's lock, and two run |
| 180 | // at once both finish with every value still opening. |
| 181 | func TestRotateSecretsSerializes(t *testing.T) { |
| 182 | cfg := testConfig(t) |
| 183 | st, repoID := storeWithSecret(t, cfg) |
| 184 | st.Close() |
| 185 | |
| 186 | unlock, err := lockKeyFile(cfg.Server.SecretKeyFile) |
| 187 | if err != nil { |
| 188 | t.Fatal(err) |
| 189 | } |
| 190 | done := make(chan error, 2) |
| 191 | go func() { done <- rotateSecrets(cfg, &bytes.Buffer{}) }() |
| 192 | go func() { done <- rotateSecrets(cfg, &bytes.Buffer{}) }() |
| 193 | select { |
| 194 | case err := <-done: |
| 195 | t.Fatalf("rotate finished while the lock was held: %v", err) |
| 196 | case <-time.After(200 * time.Millisecond): |
| 197 | } |
| 198 | unlock() |
| 199 | for range 2 { |
| 200 | select { |
| 201 | case err := <-done: |
| 202 | if err != nil { |
| 203 | t.Fatalf("rotate: %v", err) |
| 204 | } |
| 205 | case <-time.After(30 * time.Second): |
| 206 | t.Fatal("rotate never finished") |
| 207 | } |
| 208 | } |
| 209 | keys, err := seal.ReadKeys(cfg.Server.SecretKeyFile) |
| 210 | if err != nil || len(keys) != 1 { |
| 211 | t.Fatalf("key file after two rotations: %v, %v", keys, err) |
| 212 | } |
| 213 | st, err = openStore(cfg) |
| 214 | if err != nil { |
| 215 | t.Fatal(err) |
| 216 | } |
| 217 | defer st.Close() |
| 218 | if got, err := st.BuildSecrets(repoID); err != nil || got["TOKEN"] != "v1" { |
| 219 | t.Fatalf("value after two rotations: %v, %v", got, err) |
| 220 | } |
| 221 | if fi, err := os.Lstat(cfg.Server.SecretKeyFile + ".lock"); err != nil || fi.Mode().Perm() != 0o600 { |
| 222 | t.Fatalf("lock file: %v, %v", fi, err) |
| 223 | } |
| 224 | } |