diff --git a/Makefile b/Makefile index 37ad41d..dfebb48 100644 --- a/Makefile +++ b/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 diff --git a/keyring/keyring.go b/keyring/keyring.go index cce6522..4c58f8a 100644 --- a/keyring/keyring.go +++ b/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] diff --git a/keyring/keyring_linux.go b/keyring/keyring_linux.go index cca08e1..511e387 100644 --- a/keyring/keyring_linux.go +++ b/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 { diff --git a/keyring/keyring_linux_test.go b/keyring/keyring_linux_test.go index 0d9fd72..aa8bd5d 100644 --- a/keyring/keyring_linux_test.go +++ b/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") diff --git a/keyring/keyring_other.go b/keyring/keyring_other.go index 6779f29..597133b 100644 --- a/keyring/keyring_other.go +++ b/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 } diff --git a/keyring/keyring_test.go b/keyring/keyring_test.go index bbf2bc5..627fadf 100644 --- a/keyring/keyring_test.go +++ b/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) + } +}