a73x

1122d4f1

delegation: a begin nobody has signed yet survives the sweep

a73x   2026-09-02 09:42

Commit message
delegation: a begin nobody has signed yet survives the sweep

Sweep dropped every entry with no certificate on its ten-minute tick, so a
delegate_begin that a human took a while to sign lost its key, and
delegate_complete refused the certificate as if the plane had restarted.

An entry records when its keypair was minted, and Sweep keeps an unsigned
entry for defaultPendingGrace (one hour) before abandoning it; expired
delegations are still dropped. The wrong-key refusal names both ways the key
changes: a restart, or a begin left unsigned past the grace. Begin's comment,
the route Doc, the MCP delegate_begin description and docs/mcp.md say the
same, and a docs-match test derives the wording from the constant and reads
all three sites, so the number cannot drift from the prose.

Patch: 1d5ab576069812d76fdcd5e4af391cf3e60f0d2f
docs/mcp.md
Old New
@@ -66,8 +66,10 @@ before you delegate accept it too**.
66 66
67 The delegation lives in memory only. A control-plane restart drops it and you 67 The delegation lives in memory only. A control-plane restart drops it and you
68 delegate again — the public key is stable, so that is one `ssh-keygen` and one 68 delegate again — the public key is stable, so that is one `ssh-keygen` and one
69 `delegate_complete`. `GET /api/v1/delegations` reports the expiry; `DELETE` 69 `delegate_complete`. A `delegate_begin` you never complete is held for an hour
70 ends it now. 70 and then abandoned, so a certificate signed long after the fact is refused; call
71 `delegate_begin` again for the current key. `GET /api/v1/delegations` reports the
72 expiry; `DELETE` ends it now.
71 73
72 Delegation requires the tenant to have at least one registered SSH user CA 74 Delegation requires the tenant to have at least one registered SSH user CA
73 (`eitri ca upload`), because a certificate signed by a CA your guests do not 75 (`eitri ca upload`), because a certificate signed by a CA your guests do not
docs/openapi.json
Old New
@@ -1005,7 +1005,7 @@
1005 "patToken": [] 1005 "patToken": []
1006 } 1006 }
1007 ], 1007 ],
1008 "summary": "Start a delegation: returns the ephemeral public key eitri will authenticate with, the principal the certificate must carry, and the ssh-keygen command that signs it. The key is stable for the life of the process." 1008 "summary": "Start a delegation: returns the ephemeral public key eitri will authenticate with, the principal the certificate must carry, and the ssh-keygen command that signs it. The key is stable for the life of the process once delegated; a begin left unsigned for an hour is abandoned and the next call returns a new key."
1009 }, 1009 },
1010 "put": { 1010 "put": {
1011 "requestBody": { 1011 "requestBody": {
internal/mcpserver/server.go
Old New
@@ -90,8 +90,9 @@ func NewServer(t *Tools, opts Options) *mcp.Server {
90 "Ask for the public key your CA is to sign, so eitri can reach your VMs. eitri holds no signing key and "+ 90 "Ask for the public key your CA is to sign, so eitri can reach your VMs. eitri holds no signing key and "+
91 "cannot sign this itself — that is the point. Show the human the public key and the command, and wait "+ 91 "cannot sign this itself — that is the point. Show the human the public key and the command, and wait "+
92 "for them to hand back the certificate; do not try to produce it yourself. The key stays the same "+ 92 "for them to hand back the certificate; do not try to produce it yourself. The key stays the same "+
93 "until the control plane restarts — a delegation is held in memory, so after a restart call this "+ 93 "until the control plane restarts, and a begin nobody signs is abandoned after an hour — a "+
94 "tool again for the new key rather than reusing an old certificate.", 94 "delegation is held in memory, so call this tool again for the current key rather than reusing an "+
95 "old certificate.",
95 func(ctx context.Context, _ DelegateBeginIn) (BeginResult, error) { return dg.Begin(ctx) }) 96 func(ctx context.Context, _ DelegateBeginIn) (BeginResult, error) { return dg.Begin(ctx) })
96 register(s, "delegate_complete", 97 register(s, "delegate_complete",
97 "Hand back the certificate your CA signed. Certificates are public material, so passing one as an "+ 98 "Hand back the certificate your CA signed. Certificates are public material, so passing one as an "+
internal/server/api/routes.go
Old New
@@ -334,7 +334,7 @@ var routeTable = []Route{
334 Kind: KindJSON, 334 Kind: KindJSON,
335 Response: (*types.DelegationChallenge)(nil), 335 Response: (*types.DelegationChallenge)(nil),
336 Success: http.StatusOK, 336 Success: http.StatusOK,
337 Doc: "Start a delegation: returns the ephemeral public key eitri will authenticate with, the principal the certificate must carry, and the ssh-keygen command that signs it. The key is stable for the life of the process.", 337 Doc: "Start a delegation: returns the ephemeral public key eitri will authenticate with, the principal the certificate must carry, and the ssh-keygen command that signs it. The key is stable for the life of the process once delegated; a begin left unsigned for an hour is abandoned and the next call returns a new key.",
338 handler: (*API).handleBeginDelegation, 338 handler: (*API).handleBeginDelegation,
339 }, 339 },
340 { 340 {
internal/server/delegation/delegation.go
Old New
@@ -33,6 +33,12 @@ import (
33 "golang.org/x/crypto/ssh" 33 "golang.org/x/crypto/ssh"
34 ) 34 )
35 35
36 // defaultPendingGrace is how long Sweep keeps a keypair whose certificate has
37 // not come back yet. An hour is far longer than a human takes to run one
38 // ssh-keygen and far shorter than a handful of abandoned keypairs is worth
39 // worrying about.
40 const defaultPendingGrace = time.Hour
41
36 // Keyring holds one ephemeral keypair per tenant and, once delegated, the 42 // Keyring holds one ephemeral keypair per tenant and, once delegated, the
37 // certificate that makes it usable. 43 // certificate that makes it usable.
38 type Keyring struct { 44 type Keyring struct {
@@ -45,12 +51,18 @@ type Keyring struct {
45 // principals against the user being logged in as — not against the tenant. 51 // principals against the user being logged in as — not against the tenant.
46 Principal string 52 Principal string
47 53
48 mu sync.Mutex 54 mu sync.Mutex
49 tenants map[string]*entry 55 // pendingGrace is how long a begun-but-unsigned entry survives Sweep. It
56 // is unexported and set from defaultPendingGrace in New: how long eitri
57 // waits for a certificate is this package's rule, and a caller that could
58 // pin its own would be setting a security bound from the outside.
59 pendingGrace time.Duration
60 tenants map[string]*entry
50 } 61 }
51 62
52 type entry struct { 63 type entry struct {
53 key ssh.Signer // ephemeral, generated once per tenant per process 64 key ssh.Signer // ephemeral, generated once per tenant per process
65 began time.Time // when the keypair was minted; what Sweep's grace runs from
54 cert *ssh.Certificate 66 cert *ssh.Certificate
55 certSigner ssh.Signer 67 certSigner ssh.Signer
56 } 68 }
@@ -73,18 +85,20 @@ func New(now func() time.Time, principal string) *Keyring {
73 if now == nil { 85 if now == nil {
74 now = time.Now 86 now = time.Now
75 } 87 }
76 return &Keyring{Now: now, Principal: principal, tenants: map[string]*entry{}} 88 return &Keyring{Now: now, Principal: principal, pendingGrace: defaultPendingGrace, tenants: map[string]*entry{}}
77 } 89 }
78 90
79 // Begin returns the public key this tenant is to sign, generating the keypair 91 // Begin returns the public key this tenant is to sign, generating the keypair
80 // on first call and returning the SAME key every time afterwards. 92 // on first call and returning the same key on every call afterwards for as long
93 // as the entry lives.
81 // 94 //
82 // Stable-per-process is deliberate. Re-delegating after an expiry is then one 95 // Stability is deliberate. Re-delegating after an expiry is then one ssh-keygen
83 // ssh-keygen and one Complete, with no round trip to re-fetch a key that has 96 // and one Complete, with no round trip to re-fetch a key that has not changed.
84 // not changed — but only within one process: a restart generates a new key, 97 // Two things end it, and Complete's wrong-key refusal names both: a restart,
85 // which is what Complete's wrong-key refusal names. The public key is not a 98 // which mints a new key because nothing is persisted, and a begin left unsigned
86 // secret in any sense that matters: it is useless without a certificate, and 99 // past Sweep's grace, which abandons a keypair no certificate ever came back
87 // eitri cannot make itself one. 100 // for. The public key is not a secret in any sense that matters: it is useless
101 // without a certificate, and eitri cannot make itself one.
88 func (k *Keyring) Begin(tenant string) (string, error) { 102 func (k *Keyring) Begin(tenant string) (string, error) {
89 k.mu.Lock() 103 k.mu.Lock()
90 defer k.mu.Unlock() 104 defer k.mu.Unlock()
@@ -127,12 +141,14 @@ func (k *Keyring) Complete(tenant, certLine string, trusted func(ssh.PublicKey)
127 "user certificate — sign without `-h`") 141 "user certificate — sign without `-h`")
128 } 142 }
129 if !bytes.Equal(cert.Key.Marshal(), e.key.PublicKey().Marshal()) { 143 if !bytes.Equal(cert.Key.Marshal(), e.key.PublicKey().Marshal()) {
130 // Name the key that IS current. The usual cause is a certificate signed 144 // Name the key that IS current. The cause is always a certificate
131 // over a key from before a restart, and without the fingerprint to 145 // signed over a key this keyring no longer holds, and the two ways
132 // compare against there is nothing in the refusal to act on. 146 // that happens need different fixes, so the refusal names both. Without
147 // the fingerprint to compare against there is nothing to act on either.
133 return Delegation{}, fmt.Errorf("that certificate is over %s, but this tenant's current delegation key is "+ 148 return Delegation{}, fmt.Errorf("that certificate is over %s, but this tenant's current delegation key is "+
134 "%s — the key changes when the control plane restarts. Call delegate_begin again and sign the key it "+ 149 "%s — the key changes when the control plane restarts, and when a delegation is begun and left "+
135 "returns", ssh.FingerprintSHA256(cert.Key), ssh.FingerprintSHA256(e.key.PublicKey())) 150 "unsigned for longer than %s. Call delegate_begin again and sign the key it returns",
151 ssh.FingerprintSHA256(cert.Key), ssh.FingerprintSHA256(e.key.PublicKey()), k.pendingGrace)
136 } 152 }
137 153
138 ok, err = trusted(cert.SignatureKey) 154 ok, err = trusted(cert.SignatureKey)
@@ -226,16 +242,29 @@ func (k *Keyring) Revoke(tenant string) {
226 } 242 }
227 } 243 }
228 244
229 // Sweep drops every entry that is not doing anything: no certificate, or an 245 // Sweep drops every entry that is not doing anything: an expired certificate,
230 // expired one. It bounds the keyring by ACTIVE tenants rather than by every 246 // or a begin that was never signed and has outlived pendingGrace. It bounds the
231 // tenant that ever called Begin. An entry with no certificate survives until 247 // keyring by tenants that are delegating or delegated rather than by every
232 // the next sweep — it is one keypair, around a hundred bytes — so a caller who 248 // tenant that ever called Begin.
233 // takes a few minutes between Begin and Complete is not raced. 249 //
250 // The grace is there because Complete waits on a human. The tenant reads the
251 // key out of Begin, signs it with a CA that may sit on a hardware token or
252 // behind someone else's approval, and posts the certificate back minutes later.
253 // Sweep runs on a timer that knows nothing about that, so without the grace a
254 // tick landing mid-signature rotates the key underneath the caller and Complete
255 // refuses a certificate that was correct when it was signed. Waiting the grace
256 // out costs one keypair, around a hundred bytes.
234 func (k *Keyring) Sweep() { 257 func (k *Keyring) Sweep() {
235 k.mu.Lock() 258 k.mu.Lock()
236 defer k.mu.Unlock() 259 defer k.mu.Unlock()
237 for tenant, e := range k.tenants { 260 for tenant, e := range k.tenants {
238 if e.cert == nil || k.expiredLocked(e) { 261 if e.cert == nil {
262 if !k.Now().Before(e.began.Add(k.pendingGrace)) {
263 delete(k.tenants, tenant)
264 }
265 continue
266 }
267 if k.expiredLocked(e) {
239 delete(k.tenants, tenant) 268 delete(k.tenants, tenant)
240 } 269 }
241 } 270 }
@@ -254,7 +283,7 @@ func (k *Keyring) entryLocked(tenant string) (*entry, error) {
254 if err != nil { 283 if err != nil {
255 return nil, fmt.Errorf("delegation signer: %w", err) 284 return nil, fmt.Errorf("delegation signer: %w", err)
256 } 285 }
257 e := &entry{key: signer} 286 e := &entry{key: signer, began: k.Now()}
258 k.tenants[tenant] = e 287 k.tenants[tenant] = e
259 return e, nil 288 return e, nil
260 } 289 }
internal/server/delegation/delegation_doc_test.go
Old New
@@ -0,0 +1,72 @@
1 package delegation
2
3 import (
4 "os"
5 "path/filepath"
6 "runtime"
7 "strings"
8 "testing"
9 "time"
10 )
11
12 // TestDocsMatchThePendingGrace keeps the prose from drifting from the constant.
13 // defaultPendingGrace is the only place the grace is decided, and three surfaces
14 // restate it in words for readers who never see the source: the delegation
15 // section of docs/mcp.md, the delegate_begin tool description, and the route
16 // summary that becomes the OpenAPI spec and the TypeScript types. Each says the
17 // number exactly once, so this reads the real files and fails if the constant
18 // moves without them, rather than letting the promise quietly lie in three
19 // places at once.
20 func TestDocsMatchThePendingGrace(t *testing.T) {
21 want := graceProse(t, defaultPendingGrace)
22 for _, rel := range []string{
23 "docs/mcp.md",
24 "internal/mcpserver/server.go",
25 "internal/server/api/routes.go",
26 } {
27 if !strings.Contains(string(readRepoFile(t, rel)), want) {
28 t.Errorf("%s no longer tells a caller a begin is abandoned after %q — update the wording "+
29 "there and in the other two sites when defaultPendingGrace changes", rel, want)
30 }
31 }
32 }
33
34 // graceProse renders a grace the way the three surfaces say it in English. Only
35 // the durations the prose has words for are listed: changing the constant to
36 // anything else fails here first, which is the prompt to decide the wording
37 // once and then let the test above find every site that needs it.
38 func graceProse(t *testing.T, d time.Duration) string {
39 t.Helper()
40 if d == time.Hour {
41 return "an hour"
42 }
43 t.Fatalf("no documented wording for a grace of %s — write it here, then update the sites this test reads", d)
44 return ""
45 }
46
47 // readRepoFile reads a path relative to the repo root, found by walking up from
48 // this source file to the go.mod. Tests run from the package directory, so the
49 // repo root is not the working directory.
50 func readRepoFile(t *testing.T, rel string) []byte {
51 t.Helper()
52 _, self, _, ok := runtime.Caller(0)
53 if !ok {
54 t.Fatal("runtime.Caller failed")
55 }
56 dir := filepath.Dir(self)
57 for {
58 if _, err := os.Stat(filepath.Join(dir, "go.mod")); err == nil {
59 break
60 }
61 parent := filepath.Dir(dir)
62 if parent == dir {
63 t.Fatal("could not find repo root (go.mod) above " + self)
64 }
65 dir = parent
66 }
67 b, err := os.ReadFile(filepath.Join(dir, rel))
68 if err != nil {
69 t.Fatal(err)
70 }
71 return b
72 }
internal/server/delegation/delegation_test.go
Old New
@@ -349,7 +349,7 @@ func TestSweepDropsWhatIsNotBeingUsed(t *testing.T) {
349 349
350 assert.True(t, hasLive) 350 assert.True(t, hasLive)
351 assert.False(t, hasDead, "an expired delegation is dead weight") 351 assert.False(t, hasDead, "an expired delegation is dead weight")
352 assert.False(t, hasPending, "a keyring is bounded by active tenants") 352 assert.False(t, hasPending, "a begin two hours old is well past its grace, and a keyring is bounded by active tenants")
353 } 353 }
354 354
355 func TestConcurrentUseIsSafe(t *testing.T) { 355 func TestConcurrentUseIsSafe(t *testing.T) {
@@ -391,3 +391,82 @@ func TestCompleteRefusesSourceAddress(t *testing.T) {
391 _, ok := k.Signer("acme") 391 _, ok := k.Signer("acme")
392 assert.False(t, ok, "a refused certificate must leave no delegation behind") 392 assert.False(t, ok, "a refused certificate must leave no delegation behind")
393 } 393 }
394
395 // TestSweepKeepsAPendingBeginThroughTheSigningWindow proves the promise Begin
396 // makes. Signing is a human step, and the sweep ticks on its own schedule, so a
397 // tick landing between Begin and Complete must not rotate the key underneath a
398 // certificate that is already being signed.
399 func TestSweepKeepsAPendingBeginThroughTheSigningWindow(t *testing.T) {
400 clock := now
401 k := New(func() time.Time { return clock }, "ubuntu")
402
403 first, err := k.Begin("acme")
404 require.NoError(t, err)
405
406 clock = now.Add(15 * time.Minute)
407 k.Sweep()
408
409 again, err := k.Begin("acme")
410 require.NoError(t, err)
411 assert.Equal(t, first, again, "a sweep between begin and complete must not rotate the key")
412 }
413
414 // TestSweepDropsAPendingBeginPastItsGrace pins both edges of the grace: a
415 // keypair nobody signed is kept up to it and abandoned at it.
416 func TestSweepDropsAPendingBeginPastItsGrace(t *testing.T) {
417 clock := now
418 k := New(func() time.Time { return clock }, "ubuntu")
419
420 first, err := k.Begin("acme")
421 require.NoError(t, err)
422
423 clock = now.Add(defaultPendingGrace - time.Second)
424 k.Sweep()
425 kept, err := k.Begin("acme")
426 require.NoError(t, err)
427 assert.Equal(t, first, kept, "still inside the grace")
428
429 clock = now.Add(defaultPendingGrace)
430 k.Sweep()
431 fresh, err := k.Begin("acme")
432 require.NoError(t, err)
433 assert.NotEqual(t, first, fresh, "a begin left unsigned past its grace is abandoned")
434 }
435
436 // TestSweepDropsAnExpiredDelegationInsideTheBeginGrace proves the grace covers
437 // only the wait for a certificate. Once one has arrived, its own expiry is what
438 // bounds the entry, and it is dropped even though the Begin behind it is recent.
439 func TestSweepDropsAnExpiredDelegationInsideTheBeginGrace(t *testing.T) {
440 clock := now
441 k := New(func() time.Time { return clock }, "ubuntu")
442 ca := newCA(t)
443
444 pub, err := k.Begin("acme")
445 require.NoError(t, err)
446 _, err = k.Complete("acme", sign(t, ca, pub, certOpts{validBefore: now.Add(30 * time.Minute)}), trusts(ca))
447 require.NoError(t, err)
448
449 clock = now.Add(45 * time.Minute)
450 require.Less(t, clock.Sub(now), defaultPendingGrace, "the begin must still be inside its grace")
451 k.Sweep()
452
453 k.mu.Lock()
454 _, held := k.tenants["acme"]
455 k.mu.Unlock()
456 assert.False(t, held, "an expired certificate is dead weight whatever the begin grace says")
457 }
458
459 // TestTheWrongKeyRefusalNamesBothWaysTheKeyChanges checks the diagnosis covers
460 // the sweep as well as a restart; a caller told only about restarts has nothing
461 // to act on when the plane has been up for hours.
462 func TestTheWrongKeyRefusalNamesBothWaysTheKeyChanges(t *testing.T) {
463 k := New(fixedNow, "ubuntu")
464 ca := newCA(t)
465 _, err := k.Begin("acme")
466 require.NoError(t, err)
467
468 _, err = k.Complete("acme", sign(t, ca, "", certOpts{key: newCA(t).PublicKey()}), trusts(ca))
469 require.Error(t, err)
470 assert.Contains(t, err.Error(), "restarts")
471 assert.Contains(t, err.Error(), "unsigned")
472 }
web/src/lib/api-types.ts
Old New
@@ -123,7 +123,7 @@ export interface paths {
123 }; 123 };
124 }; 124 };
125 }; 125 };
126 /** Start a delegation: returns the ephemeral public key eitri will authenticate with, the principal the certificate must carry, and the ssh-keygen command that signs it. The key is stable for the life of the process. */ 126 /** Start a delegation: returns the ephemeral public key eitri will authenticate with, the principal the certificate must carry, and the ssh-keygen command that signs it. The key is stable for the life of the process once delegated; a begin left unsigned for an hour is abandoned and the next call returns a new key. */
127 post: { 127 post: {
128 parameters: { 128 parameters: {
129 query?: never; 129 query?: never;