Link keyring keys exclusively so concurrent first users share one key

Makefile

@@ -8,7 +8,9 @@ COVER_MIN := 90
 # Packages excluded from the coverage gate: pgtest exists only to be imported
 # from other repos' _test.go files, and is itself exercised by the
 # container-backed integration tests, which the gate deliberately does not run.
-COVER_EXCLUDE := /pg/pgtest/
+# keyring_linux.go is the keyctl syscall layer; its tests skip wherever seccomp
+# blocks keyctl (CI containers), so it cannot count towards the gate.
+COVER_EXCLUDE := /pg/pgtest/|/keyring/keyring_linux.go
 
 .PHONY: all build test test-all cover vet fmt lint tidy clean pre-commit patch minor major _tag
 

keyring/keyring.go

@@ -29,12 +29,6 @@ const (
 	headerLen = 1 + fpSize + nonceSize
 )
 
-// Keyring seals and unseals data with the kernel-held key named by its
-// description.
-type Keyring struct {
-	desc string
-}
-
 func fingerprint(key []byte) []byte {
 	sum := sha256.Sum256(key)
 	return sum[:fpSize]

keyring/keyring_linux.go

@@ -2,6 +2,7 @@ package keyring
 
 import (
 	"crypto/rand"
+	"encoding/binary"
 	"errors"
 	"fmt"
 	"runtime"
@@ -15,6 +16,14 @@ import (
 // shell does not.
 const keyPerm = 0x3f000000 | 0x002f0000
 
+const createAttempts = 5
+
+// Keyring seals and unseals data with the kernel-held key named by its
+// description.
+type Keyring struct {
+	desc string
+}
+
 // Open returns the keyring for desc, creating its key in the user keyring
 // (KEY_SPEC_USER_KEYRING, shared by every session of this uid until reboot) if
 // absent.
@@ -41,15 +50,15 @@ func (k *Keyring) Seal(plaintext []byte) ([]byte, error) {
 // Unseal decrypts data from Seal. It returns ErrKeyUnavailable when the key is
 // gone or differs from the sealing key, and ErrCorrupt when data is damaged.
 func (k *Keyring) Unseal(sealed []byte) ([]byte, error) {
-	key, err := k.get()
+	_, key, err := k.lookup()
 	if err != nil {
 		return nil, err
 	}
 	return unseal(key, sealed)
 }
 
-// Forget revokes the key, so everything sealed with it becomes unreadable.
-// Forgetting an absent key is not an error.
+// Forget revokes the key and unlinks it from the user keyring, so everything
+// sealed with it becomes unreadable. Forgetting an absent key is not an error.
 func (k *Keyring) Forget() error {
 	id, err := unix.KeyctlSearch(unix.KEY_SPEC_USER_KEYRING, "user", k.desc, 0)
 	if err != nil {
@@ -61,65 +70,108 @@ func (k *Keyring) Forget() error {
 	if _, err := unix.KeyctlInt(unix.KEYCTL_REVOKE, id, 0, 0, 0); err != nil {
 		return fmt.Errorf("keyring: revoke %q: %w", k.desc, err)
 	}
+	unlink(id)
 	return nil
 }
 
-func (k *Keyring) get() ([]byte, error) {
+// lookup returns the key and its id; the id is non-zero whenever the search
+// found a key, even one that turned out unusable.
+func (k *Keyring) lookup() (int, []byte, error) {
 	id, err := unix.KeyctlSearch(unix.KEY_SPEC_USER_KEYRING, "user", k.desc, 0)
 	if err != nil {
 		if missing(err) {
-			return nil, fmt.Errorf("%w: %v", ErrKeyUnavailable, err)
+			return 0, nil, fmt.Errorf("%w: %v", ErrKeyUnavailable, err)
 		}
-		return nil, fmt.Errorf("keyring: search %q: %w", k.desc, err)
+		return 0, nil, fmt.Errorf("keyring: search %q: %w", k.desc, err)
 	}
 	buf := make([]byte, 2*keySize)
 	n, err := unix.KeyctlBuffer(unix.KEYCTL_READ, id, buf, 0)
 	if err != nil {
 		if missing(err) {
-			return nil, fmt.Errorf("%w: %v", ErrKeyUnavailable, err)
+			return id, nil, fmt.Errorf("%w: %v", ErrKeyUnavailable, err)
 		}
-		return nil, fmt.Errorf("keyring: read %q: %w", k.desc, err)
+		return id, nil, fmt.Errorf("keyring: read %q: %w", k.desc, err)
 	}
 	if n != keySize {
-		return nil, fmt.Errorf("%w: key %q is %d bytes", ErrKeyUnavailable, k.desc, n)
+		return id, nil, fmt.Errorf("%w: key %q is %d bytes", ErrKeyUnavailable, k.desc, n)
 	}
-	return buf[:n], nil
+	return id, buf[:n], nil
 }
 
+// getOrCreate converges concurrent first users on one key: create links
+// exclusively, so a loser gets EEXIST and re-reads the winner's key.
 func (k *Keyring) getOrCreate() ([]byte, error) {
-	key, err := k.get()
-	if !errors.Is(err, ErrKeyUnavailable) {
-		return key, err
+	for range createAttempts {
+		id, key, err := k.lookup()
+		if !errors.Is(err, ErrKeyUnavailable) {
+			return key, err
+		}
+		// An unusable key of our description blocks the exclusive link.
+		if id != 0 {
+			unlink(id)
+		} else {
+			purgeDead()
+		}
+		key, err = k.create()
+		if !errors.Is(err, unix.EEXIST) {
+			return key, err
+		}
 	}
-	if err := k.create(); err != nil {
-		return nil, err
-	}
-	// Re-read so concurrent creators converge on whichever key was linked last.
-	return k.get()
+	return nil, fmt.Errorf("keyring: %q: key creation kept racing", k.desc)
 }
 
-// create adds the key to this thread's keyring, which is always possessed, so
-// setperm succeeds; linking it into the user keyring then replaces any stale
-// key of the same description.
-func (k *Keyring) create() error {
+// create adds a fresh key to this thread's keyring, which is always possessed,
+// so setperm succeeds, then moves it into the user keyring with MOVE_EXCL,
+// failing with EEXIST if a key of the same description is already there.
+func (k *Keyring) create() ([]byte, error) {
 	key := make([]byte, keySize)
 	if _, err := rand.Read(key); err != nil {
-		return err
+		return nil, err
 	}
 	runtime.LockOSThread()
 	defer runtime.UnlockOSThread()
 	id, err := unix.AddKey("user", k.desc, key, unix.KEY_SPEC_THREAD_KEYRING)
 	if err != nil {
-		return fmt.Errorf("keyring: add %q: %w", k.desc, err)
+		return nil, fmt.Errorf("keyring: add %q: %w", k.desc, err)
 	}
 	defer func() { _, _ = unix.KeyctlInt(unix.KEYCTL_UNLINK, id, unix.KEY_SPEC_THREAD_KEYRING, 0, 0) }()
 	if err := unix.KeyctlSetperm(id, keyPerm); err != nil {
-		return fmt.Errorf("keyring: setperm %q: %w", k.desc, err)
+		return nil, fmt.Errorf("keyring: setperm %q: %w", k.desc, err)
 	}
-	if _, err := unix.KeyctlInt(unix.KEYCTL_LINK, id, unix.KEY_SPEC_USER_KEYRING, 0, 0); err != nil {
-		return fmt.Errorf("keyring: link %q: %w", k.desc, err)
+	if _, err := unix.KeyctlInt(unix.KEYCTL_MOVE, id, unix.KEY_SPEC_THREAD_KEYRING, unix.KEY_SPEC_USER_KEYRING, unix.KEYCTL_MOVE_EXCL); err != nil {
+		if errors.Is(err, unix.EEXIST) {
+			return nil, err
+		}
+		return nil, fmt.Errorf("keyring: move %q: %w", k.desc, err)
 	}
-	return nil
+	return key, nil
+}
+
+// purgeDead unlinks revoked and expired keys from the user keyring. Their
+// description can no longer be read, so all of them go; they are inert.
+func purgeDead() {
+	buf := make([]byte, 256)
+	for {
+		n, err := unix.KeyctlBuffer(unix.KEYCTL_READ, unix.KEY_SPEC_USER_KEYRING, buf, 0)
+		if err != nil {
+			return
+		}
+		if n <= len(buf) {
+			buf = buf[:n]
+			break
+		}
+		buf = make([]byte, n)
+	}
+	for ; len(buf) >= 4; buf = buf[4:] {
+		id := int(int32(binary.NativeEndian.Uint32(buf)))
+		if _, err := unix.KeyctlString(unix.KEYCTL_DESCRIBE, id); missing(err) {
+			unlink(id)
+		}
+	}
+}
+
+func unlink(id int) {
+	_, _ = unix.KeyctlInt(unix.KEYCTL_UNLINK, id, unix.KEY_SPEC_USER_KEYRING, 0, 0)
 }
 
 func missing(err error) bool {

keyring/keyring_linux_test.go

@@ -7,6 +7,7 @@ import (
 	"os"
 	"runtime"
 	"strings"
+	"sync"
 	"testing"
 
 	"golang.org/x/sys/unix"
@@ -123,11 +124,62 @@ func TestKernelWrongSizedKeyIsReplaced(t *testing.T) {
 	if _, err := Open(k.desc); err != nil {
 		t.Fatal(err)
 	}
-	if _, err := k.get(); err != nil {
+	if _, _, err := k.lookup(); err != nil {
 		t.Fatalf("key not replaced: %v", err)
 	}
 }
 
+// Racing first users must all end up sealing under the one key that stays linked.
+func TestKernelConcurrentFirstSeal(t *testing.T) {
+	open(t) // skips where keyctl is blocked
+	const rounds, workers = 20, 8
+	for r := range rounds {
+		desc := fmt.Sprintf("golib-test:%s:%d:%d", t.Name(), os.Getpid(), r)
+		sealed := make([][]byte, workers)
+		errs := make([]error, workers)
+		var wg sync.WaitGroup
+		for w := range workers {
+			wg.Go(func() {
+				k, err := Open(desc)
+				if err == nil {
+					sealed[w], err = k.Seal([]byte("secret"))
+				}
+				errs[w] = err
+			})
+		}
+		wg.Wait()
+		k := &Keyring{desc: desc}
+		t.Cleanup(func() { _ = k.Forget() })
+		for w := range workers {
+			if errs[w] != nil {
+				t.Fatalf("round %d worker %d: %v", r, w, errs[w])
+			}
+			if got, err := k.Unseal(sealed[w]); err != nil || string(got) != "secret" {
+				t.Fatalf("round %d worker %d: Unseal = %q, %v", r, w, got, err)
+			}
+		}
+	}
+}
+
+// A key revoked behind our back still blocks an exclusive link until unlinked.
+func TestKernelExternallyRevokedKeyIsReplaced(t *testing.T) {
+	k := open(t)
+	id, _, err := k.lookup()
+	if err != nil {
+		t.Fatal(err)
+	}
+	if _, err := unix.KeyctlInt(unix.KEYCTL_REVOKE, id, 0, 0, 0); err != nil {
+		t.Fatal(err)
+	}
+	sealed, err := k.Seal([]byte("secret"))
+	if err != nil {
+		t.Fatalf("Seal after external revoke: %v", err)
+	}
+	if got, err := k.Unseal(sealed); err != nil || string(got) != "secret" {
+		t.Fatalf("Unseal = %q, %v", got, err)
+	}
+}
+
 func TestOpenEmptyDescription(t *testing.T) {
 	if _, err := Open(""); err == nil {
 		t.Fatal("Open(\"\") succeeded")

keyring/keyring_other.go

@@ -9,6 +9,9 @@ import (
 
 var errUnsupported = fmt.Errorf("keyring: kernel keyring requires linux: %w", errors.ErrUnsupported)
 
+// Keyring is a stub: the kernel key retention service is Linux-only.
+type Keyring struct{}
+
 // Open always fails: the kernel key retention service is Linux-only.
 func Open(string) (*Keyring, error) { return nil, errUnsupported }
 

keyring/keyring_test.go

@@ -2,6 +2,7 @@ package keyring
 
 import (
 	"bytes"
+	"crypto/rand"
 	"errors"
 	"testing"
 )
@@ -69,3 +70,21 @@ func TestBadKeyLength(t *testing.T) {
 		t.Fatalf("unseal err = %v, want ErrKeyUnavailable", err)
 	}
 }
+
+// The header must be bound as additional data: a valid GCM ciphertext under a
+// genuine header but without that binding is rejected.
+func TestUnsealRequiresHeaderAAD(t *testing.T) {
+	key := testKey(1)
+	gcm, err := newGCM(key)
+	if err != nil {
+		t.Fatal(err)
+	}
+	env := append([]byte{version1}, fingerprint(key)...)
+	nonce := make([]byte, nonceSize)
+	_, _ = rand.Read(nonce)
+	env = append(env, nonce...)
+	env = gcm.Seal(env, nonce, []byte("secret"), nil)
+	if _, err := unseal(key, env); !errors.Is(err, ErrCorrupt) {
+		t.Fatalf("err = %v, want ErrCorrupt", err)
+	}
+}