diff --git a/Makefile b/Makefile index dfebb48..37ad41d 100644 --- a/Makefile +++ b/Makefile @@ -8,9 +8,7 @@ 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. -# 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 +COVER_EXCLUDE := /pg/pgtest/ .PHONY: all build test test-all cover vet fmt lint tidy clean pre-commit patch minor major _tag diff --git a/keyring/keyring_linux.go b/keyring/keyring_linux.go index 511e387..c535796 100644 --- a/keyring/keyring_linux.go +++ b/keyring/keyring_linux.go @@ -80,7 +80,7 @@ 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 0, nil, fmt.Errorf("%w: %v", ErrKeyUnavailable, err) + return 0, nil, fmt.Errorf("%w: %w", ErrKeyUnavailable, err) } return 0, nil, fmt.Errorf("keyring: search %q: %w", k.desc, err) } @@ -109,7 +109,7 @@ func (k *Keyring) getOrCreate() ([]byte, error) { // An unusable key of our description blocks the exclusive link. if id != 0 { unlink(id) - } else { + } else if errors.Is(err, unix.EKEYREVOKED) || errors.Is(err, unix.EKEYEXPIRED) { purgeDead() } key, err = k.create() @@ -147,8 +147,9 @@ func (k *Keyring) create() ([]byte, error) { 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. +// purgeDead unlinks every revoked or expired key in the user keyring: a dead key +// cannot be described, so ours cannot be singled out. Call it only after the +// search hit a dead key of our description. func purgeDead() { buf := make([]byte, 256) for { diff --git a/keyring/keyring_linux_test.go b/keyring/keyring_linux_test.go index aa8bd5d..7f2bbb5 100644 --- a/keyring/keyring_linux_test.go +++ b/keyring/keyring_linux_test.go @@ -114,7 +114,10 @@ func TestKernelForget(t *testing.T) { func TestKernelWrongSizedKeyIsReplaced(t *testing.T) { k := open(t) - sealed, _ := k.Seal([]byte("secret")) + sealed, err := k.Seal([]byte("secret")) + if err != nil { + t.Fatal(err) + } if _, err := unix.AddKey("user", k.desc, []byte("short"), unix.KEY_SPEC_USER_KEYRING); err != nil { t.Fatal(err) } @@ -180,6 +183,28 @@ func TestKernelExternallyRevokedKeyIsReplaced(t *testing.T) { } } +// Another application's dead key in the user keyring is not ours to unlink. +func TestKernelForeignDeadKeySurvivesOpen(t *testing.T) { + open(t) // skips where keyctl is blocked + id, err := unix.AddKey("user", fmt.Sprintf("golib-test-foreign:%s:%d", t.Name(), os.Getpid()), []byte("x"), unix.KEY_SPEC_USER_KEYRING) + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { unlink(id) }) + if _, err := unix.KeyctlInt(unix.KEYCTL_REVOKE, id, 0, 0, 0); err != nil { + t.Fatal(err) + } + k, err := Open(fmt.Sprintf("golib-test:%s:%d:fresh", t.Name(), os.Getpid())) + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { _ = k.Forget() }) + // Unlinking an id that is no longer linked fails with ENOENT. + if _, err := unix.KeyctlInt(unix.KEYCTL_UNLINK, id, unix.KEY_SPEC_USER_KEYRING, 0, 0); err != nil { + t.Fatalf("foreign dead key was unlinked: %v", err) + } +} + func TestOpenEmptyDescription(t *testing.T) { if _, err := Open(""); err == nil { t.Fatal("Open(\"\") succeeded")