Sign in

Search issue titles and bodies, change requests and comments. Private repositories you cannot read are not included.
vole
Clone

Read only, no credentials:

https://rodent.software/vole.git

Write over SSH with a registered key:

git clone ssh://git@rodent.software:2222/vole.git

A free and open-source forge that lives in your Git repo.

Replace the git-backed roster with a trusted keys directory

197ecaa29d992ecce2b6078803f3512f565f3672 · Keenan Nemetz · 18d ago · parent a5f19b3c 

Replace the git-backed roster with a trusted keys directory

The roster was 476 lines of CAS retries, tree building and snapshot reloading to
store what amounts to username -> authorized_keys lines. Two of the three fields
in meta.json were unused: ACLs key on username, never on id, and nothing ever set
disabled. The git version was paying for history and tamper evidence that
nothing consumed.

Now:

  data/trusted_keys/keenan.keys    # one authorized_keys line per device

This is the AuthorizedKeysFile convention, so ssh-keygen -lf, ansible, rsync and
an editor all work on it, and dropping a key in by hand is a supported way to add
a user because the running instance re-reads the directory every five seconds.
Adds are O_APPEND, so two processes cannot lose each other's writes; removes are
read-modify-rename and assume a single operator. A file that does not parse, or
the same key under two names, still refuses startup rather than silently changing
who can authenticate.

What the directory could not provide on its own is an audit trail, so key changes
append a line to data/audit.log: timestamp, action, user, and key fingerprint.
That is the git log we deleted, at about 15 lines instead of a commit graph. If a
user must be blocked rather than removed, their keys are removed; there is no
disabled flag until something needs one.

Identity is now ~200 lines instead of 476, and the lost-update class of bug that
the CAS loop made possible is gone with the loop.

Verified by hand: fresh init shows data/ only holding audit.log, index.db, repos
and trusted_keys; a key appended to the file by hand starts working within the
poll interval without a restart; removing a key by CLI stops that key
authenticating; audit.log records both.

internal/identity/identity_test.go +170−132

@@ -12,18 +12,18 @@ import (
12 12 "golang.org/x/crypto/ssh"
13 13 )
14 14
15 func newStore(t *testing.T) (*Store, string) {
15 func newStore(t *testing.T) *Store {
16 16 t.Helper()
17 dir := t.TempDir()
18 sys := filepath.Join(dir, "system.git")
19 if err := Init(sys); err != nil {
17 dir := filepath.Join(t.TempDir(), "trusted_keys")
18 if err := Init(dir); err != nil {
20 19 t.Fatal(err)
21 20 }
22 s, err := Open(sys)
21 s, err := Open(dir)
23 22 if err != nil {
24 23 t.Fatal(err)
25 24 }
26 return s, sys
25 s.AuditPath = filepath.Join(filepath.Dir(dir), "audit.log")
26 return s
27 27 }
28 28
29 29 func pubLine(t *testing.T) (string, string) {
@@ -39,11 +39,27 @@ func pubLine(t *testing.T) (string, string) {
39 39 return strings.TrimSpace(string(ssh.MarshalAuthorizedKey(k))), ssh.FingerprintSHA256(k)
40 40 }
41 41
42 func TestEmptyRosterLoads(t *testing.T) {
43 s := newStore(t)
44 if got := len(s.Snapshot().Users); got != 0 {
45 t.Fatalf("users = %d, want 0", got)
46 }
47 }
48
49 func TestMissingDirectoryIsAnError(t *testing.T) {
50 if _, err := Open(filepath.Join(t.TempDir(), "nope")); err == nil {
51 t.Fatal("Open succeeded on a missing directory")
52 }
53 }
54
42 55 func TestCreateUserAndAddKey(t *testing.T) {
43 s, _ := newStore(t)
56 s := newStore(t)
44 57 if err := s.Create("keenan"); err != nil {
45 58 t.Fatal(err)
46 59 }
60 if _, ok := s.Snapshot().Users["keenan"]; !ok {
61 t.Fatal("user missing after create")
62 }
47 63 line, wantFP := pubLine(t)
48 64 fp, err := s.AddKey("keenan", line, "test")
49 65 if err != nil {
@@ -52,17 +68,53 @@ func TestCreateUserAndAddKey(t *testing.T) {
52 68 if fp != wantFP {
53 69 t.Fatalf("fingerprint = %s, want %s", fp, wantFP)
54 70 }
55 name, ok := s.Lookup(fp)
56 if !ok || name != "keenan" {
57 t.Fatalf("Lookup(%s) = %q,%v", fp, name, ok)
71 if name, ok := s.Lookup(fp); !ok || name != "keenan" {
72 t.Fatalf("Lookup = %q,%v", name, ok)
73 }
74 data, err := os.ReadFile(filepath.Join(s.Dir, "keenan.keys"))
75 if err != nil {
76 t.Fatal(err)
77 }
78 if !strings.Contains(string(data), line) {
79 t.Fatalf("key not written verbatim:\n%s", data)
80 }
81 if !strings.HasPrefix(string(data), "#") {
82 t.Fatal("file has no explanatory header")
83 }
84 }
85
86 func TestMultipleKeysPerUser(t *testing.T) {
87 s := newStore(t)
88 if err := s.Create("keenan"); err != nil {
89 t.Fatal(err)
90 }
91 var fps []string
92 for i := 0; i < 3; i++ {
93 line, fp := pubLine(t)
94 if _, err := s.AddKey("keenan", line, "test"); err != nil {
95 t.Fatal(err)
96 }
97 fps = append(fps, fp)
98 }
99 if got := len(s.Snapshot().Users["keenan"].Keys); got != 3 {
100 t.Fatalf("keys = %d, want 3", got)
58 101 }
59 if got := len(s.Snapshot().Users["keenan"].Keys); got != 1 {
60 t.Fatalf("key count = %d", got)
102 reopened, err := Open(s.Dir)
103 if err != nil {
104 t.Fatal(err)
105 }
106 if got := len(reopened.Snapshot().Users["keenan"].Keys); got != 3 {
107 t.Fatalf("keys after reopen = %d, want 3", got)
108 }
109 for _, fp := range fps {
110 if _, ok := reopened.Lookup(fp); !ok {
111 t.Fatalf("key %s lost across reopen", fp)
112 }
61 113 }
62 114 }
63 115
64 116 func TestDuplicateKeyAcrossUsersRejected(t *testing.T) {
65 s, _ := newStore(t)
117 s := newStore(t)
66 118 for _, n := range []string{"alice", "bob"} {
67 119 if err := s.Create(n); err != nil {
68 120 t.Fatal(err)
@@ -73,60 +125,119 @@ func TestDuplicateKeyAcrossUsersRejected(t *testing.T) {
73 125 t.Fatal(err)
74 126 }
75 127 if _, err := s.AddKey("bob", line, "test"); err == nil {
76 t.Fatal("the same key was accepted for a second user")
128 t.Fatal("same key accepted for a second user")
77 129 } else if !strings.Contains(err.Error(), "already registered") {
78 130 t.Fatalf("unclear error: %v", err)
79 131 }
80 132 }
81 133
82 func TestRemoveKeyRevokesAccess(t *testing.T) {
83 s, _ := newStore(t)
84 if err := s.Create("keenan"); err != nil {
134 // TestDuplicateKeyOnDiskFailsStartup covers a hand-edited directory: two files
135 // claiming the same key is ambiguous, so the roster must refuse to load.
136 func TestDuplicateKeyOnDiskFailsStartup(t *testing.T) {
137 s := newStore(t)
138 for _, n := range []string{"alice", "bob"} {
139 if err := s.Create(n); err != nil {
140 t.Fatal(err)
141 }
142 }
143 line, _ := pubLine(t)
144 if _, err := s.AddKey("alice", line, "test"); err != nil {
85 145 t.Fatal(err)
86 146 }
87 line, fp := pubLine(t)
88 if _, err := s.AddKey("keenan", line, "test"); err != nil {
147 // Hand-copy the same key into bob's file, bypassing AddKey.
148 if err := os.WriteFile(filepath.Join(s.Dir, "bob.keys"), []byte(line+"\n"), 0o644); err != nil {
89 149 t.Fatal(err)
90 150 }
91 if err := s.RemoveKey("keenan", fp, "test"); err != nil {
151 if _, err := Open(s.Dir); err == nil {
152 t.Fatal("ambiguous roster loaded; it must refuse to start")
153 }
154 }
155
156 func TestRemoveKey(t *testing.T) {
157 s := newStore(t)
158 if err := s.Create("keenan"); err != nil {
92 159 t.Fatal(err)
93 160 }
94 if _, ok := s.Lookup(fp); ok {
161 keepLine, keepFP := pubLine(t)
162 dropLine, dropFP := pubLine(t)
163 for _, l := range []string{keepLine, dropLine} {
164 if _, err := s.AddKey("keenan", l, "test"); err != nil {
165 t.Fatal(err)
166 }
167 }
168 if err := s.RemoveKey("keenan", dropFP, "test"); err != nil {
169 t.Fatal(err)
170 }
171 if _, ok := s.Lookup(dropFP); ok {
95 172 t.Fatal("revoked key still authenticates")
96 173 }
174 if _, ok := s.Lookup(keepFP); !ok {
175 t.Fatal("removing one key took the other with it")
176 }
177 data, err := os.ReadFile(filepath.Join(s.Dir, "keenan.keys"))
178 if err != nil {
179 t.Fatal(err)
180 }
181 if strings.Contains(string(data), dropLine) {
182 t.Fatal("revoked key still on disk")
183 }
184 if !strings.Contains(string(data), keepLine) {
185 t.Fatal("kept key vanished from disk")
186 }
97 187 }
98 188
99 func TestTreeLayoutIsTheSchema(t *testing.T) {
100 s, _ := newStore(t)
189 func TestUnparsableLineFailsStartup(t *testing.T) {
190 s := newStore(t)
101 191 if err := s.Create("keenan"); err != nil {
102 192 t.Fatal(err)
103 193 }
104 line, _ := pubLine(t)
105 if _, err := s.AddKey("keenan", line, "test"); err != nil {
194 if err := os.WriteFile(filepath.Join(s.Dir, "keenan.keys"), []byte("this is not a key\n"), 0o644); err != nil {
195 t.Fatal(err)
196 }
197 if _, err := Open(s.Dir); err == nil {
198 t.Fatal("roster with an unparsable key loaded; it must refuse to start")
199 }
200 }
201
202 func TestNotesAndBlanksIgnored(t *testing.T) {
203 s := newStore(t)
204 line, fp := pubLine(t)
205 content := "# a note\n\n \n" + line + "\n# trailing note\n"
206 if err := os.WriteFile(filepath.Join(s.Dir, "keenan.keys"), []byte(content), 0o644); err != nil {
106 207 t.Fatal(err)
107 208 }
108 entries, err := s.Git.LsTreeR(UsersRef)
209 re, err := Open(s.Dir)
109 210 if err != nil {
110 211 t.Fatal(err)
111 212 }
112 var sawMeta, sawKey bool
113 for _, e := range entries {
114 if e.Name == "users/keenan/meta.json" {
115 sawMeta = true
116 }
117 if strings.HasPrefix(e.Name, "users/keenan/keys/") && strings.HasSuffix(e.Name, ".pub") {
118 sawKey = true
213 if _, ok := re.Lookup(fp); !ok {
214 t.Fatal("key on a commented file was not loaded")
215 }
216 if got := len(re.Snapshot().Users["keenan"].Keys); got != 1 {
217 t.Fatalf("keys = %d, want 1", got)
218 }
219 }
220
221 func TestInvalidNamesRefused(t *testing.T) {
222 s := newStore(t)
223 for _, name := range []string{"Keenan", "has space", "a", "with/slash", "trailing."} {
224 if err := s.Create(name); err == nil {
225 t.Fatalf("%q accepted as a username", name)
119 226 }
120 227 }
121 if !sawMeta || !sawKey {
122 t.Fatalf("unexpected tree: %+v", entries)
228 // A file named with an invalid user must also fail to load.
229 if err := os.WriteFile(filepath.Join(s.Dir, "Bad Name.keys"), []byte("x\n"), 0o644); err != nil {
230 t.Fatal(err)
231 }
232 if _, err := Open(s.Dir); err == nil {
233 t.Fatal("invalid filename in the roster directory was accepted")
123 234 }
124 235 }
125 236
126 // TestConcurrentAddsAllLand is the CAS guarantee: two stores editing the roster
127 // at once must not lose each other's writes.
237 // TestConcurrentAddsAllLand relies on append semantics: no read-modify-write, so
238 // there is no lost-update window.
128 239 func TestConcurrentAddsAllLand(t *testing.T) {
129 s, sys := newStore(t)
240 s := newStore(t)
130 241 if err := s.Create("keenan"); err != nil {
131 242 t.Fatal(err)
132 243 }
@@ -135,22 +246,13 @@ func TestConcurrentAddsAllLand(t *testing.T) {
135 246 for i := range lines {
136 247 lines[i], _ = pubLine(t)
137 248 }
138
139 other, err := Open(sys)
140 if err != nil {
141 t.Fatal(err)
142 }
143 249 var wg sync.WaitGroup
144 250 errs := make([]error, n)
145 251 for i := 0; i < n; i++ {
146 252 wg.Add(1)
147 253 go func(i int) {
148 254 defer wg.Done()
149 store := s
150 if i%2 == 1 {
151 store = other
152 }
153 _, errs[i] = store.AddKey("keenan", lines[i], "test")
255 _, errs[i] = s.AddKey("keenan", lines[i], "test")
154 256 }(i)
155 257 }
156 258 wg.Wait()
@@ -159,114 +261,50 @@ func TestConcurrentAddsAllLand(t *testing.T) {
159 261 t.Fatalf("add %d: %v", i, err)
160 262 }
161 263 }
162 final, err := Open(sys)
264 final, err := Open(s.Dir)
163 265 if err != nil {
164 266 t.Fatal(err)
165 267 }
166 268 if got := len(final.Snapshot().Users["keenan"].Keys); got != n {
167 t.Fatalf("after concurrent adds: %d keys, want %d (a CAS retry lost a write)", got, n)
269 t.Fatalf("after concurrent adds: %d keys, want %d", got, n)
168 270 }
169 271 }
170 272
171 func TestReloadSeesExternalWrites(t *testing.T) {
172 s, sys := newStore(t)
273 func TestAuditLogRecordsChanges(t *testing.T) {
274 s := newStore(t)
173 275 if err := s.Create("keenan"); err != nil {
174 276 t.Fatal(err)
175 277 }
176 278 line, fp := pubLine(t)
177
178 other, err := Open(sys)
179 if err != nil {
180 t.Fatal(err)
181 }
182 if _, err := other.AddKey("keenan", line, "test"); err != nil {
183 t.Fatal(err)
184 }
185 if _, ok := s.Lookup(fp); ok {
186 t.Fatal("stale snapshot saw a key it should not have")
187 }
188 if err := s.Reload(); err != nil {
279 if _, err := s.AddKey("keenan", line, "operator"); err != nil {
189 280 t.Fatal(err)
190 281 }
191 if _, ok := s.Lookup(fp); !ok {
192 t.Fatal("Reload did not pick up the new key")
193 }
194 }
195
196 // TestCorruptRosterFailsLoudly covers the corruption that is actually
197 // reachable. git itself refuses to point a branch ref at a non-commit, so the
198 // failure mode to guard is a commit whose roster files do not parse: without a
199 // hard error, Open would boot an instance with no usable users.
200 func TestCorruptRosterFailsLoudly(t *testing.T) {
201 s, sys := newStore(t)
202 if err := s.Create("keenan"); err != nil {
282 if err := s.RemoveKey("keenan", fp, "operator"); err != nil {
203 283 t.Fatal(err)
204 284 }
205 old, err := s.Git.RevParseRaw(UsersRef)
285 data, err := os.ReadFile(s.AuditPath)
206 286 if err != nil {
207 287 t.Fatal(err)
208 288 }
209 tree, err := buildTree(s.Git, map[string][]byte{
210 "users/broken/meta.json": []byte("{ this is not json"),
211 })
212 if err != nil {
213 t.Fatal(err)
214 }
215 commit, err := s.Git.CommitTree(tree, old, "corrupt roster")
216 if err != nil {
217 t.Fatal(err)
218 }
219 if err := s.Git.UpdateRef(UsersRef, commit); err != nil {
220 t.Fatal(err)
221 }
222 if _, err := Open(sys); err == nil {
223 t.Fatal("Open succeeded on an unparsable roster; it must refuse to start")
224 }
225 }
226
227 // TestBranchRefCannotPointAtANonCommit documents the property we get for free by
228 // storing the roster on a branch rather than a custom ref.
229 func TestBranchRefCannotPointAtANonCommit(t *testing.T) {
230 s, _ := newStore(t)
231 if err := s.Create("keenan"); err != nil {
232 t.Fatal(err)
233 }
234 blob, err := s.Git.HashObject([]byte("not a commit"))
235 if err != nil {
236 t.Fatal(err)
289 log := string(data)
290 for _, want := range []string{"identity.create", "keenan", "identity.add-key", "identity.remove-key", fp} {
291 if !strings.Contains(log, want) {
292 t.Fatalf("audit log missing %q:\n%s", want, log)
293 }
237 294 }
238 if err := s.Git.UpdateRef(UsersRef, blob); err == nil {
239 t.Fatal("git allowed a branch ref to point at a blob")
295 if got := strings.Count(log, "\n"); got != 3 {
296 t.Fatalf("audit lines = %d, want 3:\n%s", got, log)
240 297 }
241 298 }
242 299
243 func TestDisabledUserCannotAuthenticate(t *testing.T) {
244 s, sys := newStore(t)
300 func TestAuditFailureDoesNotBlockChanges(t *testing.T) {
301 s := newStore(t)
302 s.AuditPath = filepath.Join(t.TempDir(), "missing-dir", "audit.log")
245 303 if err := s.Create("keenan"); err != nil {
246 t.Fatal(err)
304 t.Fatalf("create failed because the audit log was unwritable: %v", err)
247 305 }
248 line, fp := pubLine(t)
306 line, _ := pubLine(t)
249 307 if _, err := s.AddKey("keenan", line, "test"); err != nil {
250 t.Fatal(err)
251 }
252 u := s.Snapshot().Users["keenan"]
253 u.Disabled = true
254 if _, ok := s.Lookup(fp); ok {
255 t.Fatal("disabled user still authenticates")
256 }
257 _ = sys
258 }
259
260 func TestInitIsIdempotent(t *testing.T) {
261 dir := t.TempDir()
262 sys := filepath.Join(dir, "system.git")
263 if err := Init(sys); err != nil {
264 t.Fatal(err)
265 }
266 if err := Init(sys); err != nil {
267 t.Fatalf("second Init: %v", err)
268 }
269 if _, err := os.Stat(filepath.Join(sys, "HEAD")); err != nil {
270 t.Fatal(err)
308 t.Fatalf("add-key failed because the audit log was unwritable: %v", err)
271 309 }
272 310 }