fix: run DWH verification with read-only registry access

This commit is contained in:
User
2026-08-21 02:14:50 +02:00
parent 419c3440d7
commit 134dc1977c
6 changed files with 291 additions and 14 deletions
+61 -5
View File
@@ -47,6 +47,8 @@ var (
ErrConflict = errors.New("credential record already exists")
// ErrIntegrity means an unsafe or malformed registry object was observed.
ErrIntegrity = errors.New("registry integrity failure")
// ErrReadOnly means a runtime reader was asked to mutate registry state.
ErrReadOnly = errors.New("registry is read-only")
)
// PublicRecord is the redacted inventory form of a persisted record.
@@ -64,11 +66,12 @@ type PublicRecord struct {
// Store owns protected descriptors for one registry root.
type Store struct {
mu sync.RWMutex
now func() time.Time
root *securefile.Dir
active *securefile.Dir
revoked *securefile.Dir
mu sync.RWMutex
now func() time.Time
root *securefile.Dir
active *securefile.Dir
revoked *securefile.Dir
readOnly bool
}
type storedRecord struct {
@@ -94,9 +97,56 @@ func Open(root string) (*Store, error) {
_ = rootDir.Close()
return nil, integrity(err)
}
lock, err := rootDir.Lock(registryLockName)
if err != nil {
_ = revoked.Close()
_ = active.Close()
_ = rootDir.Close()
return nil, integrity(err)
}
if err := lock.Close(); err != nil {
_ = revoked.Close()
_ = active.Close()
_ = rootDir.Close()
return nil, integrity(err)
}
return &Store{root: rootDir, active: active, revoked: revoked, now: time.Now}, nil
}
// OpenReadOnly opens only a complete preprovisioned protected registry. It
// neither creates registry paths nor permits mutations through the Store.
func OpenReadOnly(root string) (*Store, error) {
rootDir, err := securefile.OpenDir(root)
if err != nil {
return nil, integrity(err)
}
active, err := rootDir.OpenDir(string(StateActive))
if err != nil {
_ = rootDir.Close()
return nil, integrity(err)
}
revoked, err := rootDir.OpenDir(string(StateRevoked))
if err != nil {
_ = active.Close()
_ = rootDir.Close()
return nil, integrity(err)
}
lock, err := rootDir.LockShared(registryLockName)
if err != nil {
_ = revoked.Close()
_ = active.Close()
_ = rootDir.Close()
return nil, integrity(err)
}
if err := lock.Close(); err != nil {
_ = revoked.Close()
_ = active.Close()
_ = rootDir.Close()
return nil, integrity(err)
}
return &Store{root: rootDir, active: active, revoked: revoked, now: time.Now, readOnly: true}, nil
}
// Close closes descriptors held by the store.
func (s *Store) Close() error {
if s == nil {
@@ -119,6 +169,9 @@ func (s *Store) Close() error {
// Add validates and atomically publishes one active record. Existing active or
// revoked records cannot be overwritten or resurrected.
func (s *Store) Add(value record.Record) error {
if s != nil && s.readOnly {
return ErrReadOnly
}
if err := validateForState(value, StateActive); err != nil {
return err
}
@@ -261,6 +314,9 @@ func (s *Store) listUnlocked() ([]PublicRecord, error) {
// Revoke publishes a validated revoked record and fsyncs it before removing the
// active record. If deletion then fails, Find still returns ErrRevoked.
func (s *Store) Revoke(keyID, reason string, at time.Time) error {
if s != nil && s.readOnly {
return ErrReadOnly
}
if !validKeyID(keyID) {
return ErrNotFound
}
@@ -112,6 +112,82 @@ func TestOpenRejectsSymlinkedRegistryRootAndDirectories(t *testing.T) {
})
}
func TestOpenReadOnlyRequiresCompletePreprovisionedRegistryAndNeverCreates(t *testing.T) {
root := t.TempDir()
before, err := os.ReadDir(root)
if err != nil {
t.Fatalf("ReadDir(before) error = %v", err)
}
if _, err := OpenReadOnly(root); err == nil {
t.Fatal("OpenReadOnly(incomplete) error = nil, want refusal")
}
after, err := os.ReadDir(root)
if err != nil {
t.Fatalf("ReadDir(after) error = %v", err)
}
if !reflect.DeepEqual(before, after) {
t.Fatalf("OpenReadOnly(incomplete) changed root entries: before=%v after=%v", before, after)
}
admin := openStore(t, root)
item := syntheticRecord(13)
if err := admin.Add(item); err != nil {
t.Fatalf("Add() error = %v", err)
}
if err := admin.Close(); err != nil {
t.Fatalf("Close(admin) error = %v", err)
}
for _, path := range []string{root, filepath.Join(root, "active"), filepath.Join(root, "revoked")} {
if err := os.Chmod(path, 0o750|os.ModeSetgid); err != nil {
t.Fatalf("Chmod(%q, 2750) error = %v", path, err)
}
}
if info, err := os.Stat(filepath.Join(root, registryLockName)); err != nil || info.Mode().Perm() != 0o640 {
t.Fatalf("registry lock = (%v, %v), want 0640", info, err)
}
entriesBefore := registryEntryModes(t, root)
reader, err := OpenReadOnly(root)
if err != nil {
t.Fatalf("OpenReadOnly() error = %v", err)
}
defer reader.Close()
if found, err := reader.Find(item.KeyID); err != nil || found.KeyID != item.KeyID {
t.Fatalf("Find() = (%#v, %v), want protected record", found, err)
}
if err := reader.Check(); err != nil {
t.Fatalf("Check() error = %v", err)
}
if err := reader.Add(syntheticRecord(14)); err == nil {
t.Fatal("read-only Add() error = nil, want refusal")
}
if err := reader.Revoke(item.KeyID, "synthetic", item.CreatedAt.Add(time.Hour)); err == nil {
t.Fatal("read-only Revoke() error = nil, want refusal")
}
if got := registryEntryModes(t, root); !reflect.DeepEqual(got, entriesBefore) {
t.Fatalf("read-only operations changed registry entries: before=%v after=%v", entriesBefore, got)
}
}
func registryEntryModes(t *testing.T, root string) []string {
t.Helper()
var result []string
for _, directory := range []string{root, filepath.Join(root, "active"), filepath.Join(root, "revoked")} {
entries, err := os.ReadDir(directory)
if err != nil {
t.Fatalf("ReadDir(%q) error = %v", directory, err)
}
for _, entry := range entries {
info, err := entry.Info()
if err != nil {
t.Fatalf("Info(%q) error = %v", entry.Name(), err)
}
result = append(result, filepath.Join(directory, entry.Name())+":"+info.Mode().String())
}
}
sort.Strings(result)
return result
}
func TestCheckRejectsUnsafeModesAndSymlinkedRecords(t *testing.T) {
t.Run("unsafe active mode", func(t *testing.T) {
root := t.TempDir()
@@ -288,16 +288,38 @@ func (d *Dir) Names() ([]string, error) {
return names, nil
}
// Lock opens or creates a protected 0600 lock file and acquires an exclusive
// Lock opens or creates a protected 0640 lock file and acquires an exclusive
// advisory lock. Close releases the lock and descriptor.
func (d *Dir) Lock(name string) (*Lock, error) {
return d.lock(name, syscall.LOCK_EX)
}
// LockShared opens or creates a protected 0600 lock file and acquires a shared
// advisory lock. Close releases the lock and descriptor.
// LockShared opens an existing protected 0640 lock read-only and acquires a
// shared advisory lock. It never creates or changes registry state.
func (d *Dir) LockShared(name string) (*Lock, error) {
return d.lock(name, syscall.LOCK_SH)
if err := validName(name); err != nil {
return nil, err
}
if err := d.check(); err != nil {
return nil, err
}
fd, err := syscall.Openat(d.fd, name, syscall.O_RDONLY|syscall.O_NOFOLLOW|syscall.O_CLOEXEC, 0)
if err != nil {
return nil, err
}
if err := validateRegularFD(fd, 0o640); err != nil {
_ = syscall.Close(fd)
return nil, err
}
if err := compareWithLstat(fd, filepath.Join(d.path, name), false); err != nil {
_ = syscall.Close(fd)
return nil, err
}
if err := syscall.Flock(fd, syscall.LOCK_SH); err != nil {
_ = syscall.Close(fd)
return nil, err
}
return &Lock{fd: fd}, nil
}
func (d *Dir) lock(name string, lockMode int) (*Lock, error) {
@@ -310,14 +332,14 @@ func (d *Dir) lock(name string, lockMode int) (*Lock, error) {
for {
fd, err := syscall.Openat(d.fd, name, syscall.O_RDWR|syscall.O_NOFOLLOW|syscall.O_CLOEXEC, 0)
if errors.Is(err, syscall.ENOENT) {
fd, err = syscall.Openat(d.fd, name, syscall.O_RDWR|syscall.O_CREAT|syscall.O_EXCL|syscall.O_NOFOLLOW|syscall.O_CLOEXEC, 0o600)
fd, err = syscall.Openat(d.fd, name, syscall.O_RDWR|syscall.O_CREAT|syscall.O_EXCL|syscall.O_NOFOLLOW|syscall.O_CLOEXEC, 0o640)
if errors.Is(err, syscall.EEXIST) {
continue
}
if err != nil {
return nil, err
}
if err := syscall.Fchmod(fd, 0o600); err != nil {
if err := syscall.Fchmod(fd, 0o640); err != nil {
_ = syscall.Close(fd)
return nil, err
}
@@ -329,7 +351,7 @@ func (d *Dir) lock(name string, lockMode int) (*Lock, error) {
if err != nil {
return nil, err
}
if err := validateRegularFD(fd, 0o600); err != nil {
if err := validateRegularFD(fd, 0o640); err != nil {
_ = syscall.Close(fd)
return nil, err
}
@@ -495,7 +517,7 @@ func validateDirectoryFD(fd int) error {
if stat.Mode&syscall.S_IFMT != syscall.S_IFDIR {
return fmt.Errorf("%w: not a directory", ErrUnsafe)
}
if stat.Mode&0o7022 != 0 {
if stat.Mode&0o5022 != 0 {
return fmt.Errorf("%w: unsafe directory mode %04o", ErrUnsafe, stat.Mode&0o7777)
}
return nil
@@ -93,6 +93,33 @@ func TestProtectedPathsRejectSymlinks(t *testing.T) {
}
func TestProtectedPathsRejectUnsafeModes(t *testing.T) {
t.Run("setgid protected directories", func(t *testing.T) {
root := t.TempDir()
if err := os.Chmod(root, 0o750|os.ModeSetgid); err != nil {
t.Fatalf("Chmod(root) error = %v", err)
}
dir, err := OpenDir(root)
if err != nil {
t.Fatalf("OpenDir(2750 root) error = %v", err)
}
t.Cleanup(func() { _ = dir.Close() })
for _, mode := range []os.FileMode{
0o750 | os.ModeSetuid,
0o750 | os.ModeSetgid | os.ModeSetuid,
0o750 | os.ModeSticky,
0o770 | os.ModeSetgid,
0o752 | os.ModeSetgid,
} {
path := t.TempDir()
if err := os.Chmod(path, mode); err != nil {
t.Fatalf("Chmod(%v) error = %v", mode, err)
}
if _, err := OpenDir(path); err == nil {
t.Fatalf("OpenDir(%v) error = nil, want unsafe-mode refusal", mode)
}
}
})
t.Run("root", func(t *testing.T) {
root := t.TempDir()
if err := os.Chmod(root, 0o770); err != nil {
@@ -238,6 +265,22 @@ func TestLockSharedAllowsReadersAndBlocksExclusiveWriter(t *testing.T) {
t.Fatal("Dir lacks shared advisory locking")
}
if _, err := firstLocker.LockShared("registry.lock"); err == nil {
t.Fatal("LockShared(missing) error = nil, want preprovisioned-lock refusal")
}
if _, err := os.Lstat(filepath.Join(root, "registry.lock")); !errors.Is(err, os.ErrNotExist) {
t.Fatalf("LockShared(missing) created lock: %v", err)
}
bootstrap, err := third.Lock("registry.lock")
if err != nil {
t.Fatalf("Lock(bootstrap) error = %v", err)
}
if err := bootstrap.Close(); err != nil {
t.Fatalf("Close(bootstrap) error = %v", err)
}
if info, err := os.Stat(filepath.Join(root, "registry.lock")); err != nil || info.Mode().Perm() != 0o640 {
t.Fatalf("bootstrapped lock mode = (%v, %v), want 0640", info, err)
}
firstShared, err := firstLocker.LockShared("registry.lock")
if err != nil {
t.Fatalf("LockShared(first) error = %v", err)
+27 -1
View File
@@ -48,6 +48,18 @@ var socketOwner = func(path string) (uint32, error) {
return stat.Uid, nil
}
var socketParentOwner = func(path string) (uint32, error) {
info, err := os.Lstat(path)
if err != nil {
return 0, err
}
stat, ok := info.Sys().(*syscall.Stat_t)
if !ok {
return 0, fmt.Errorf("socket parent owner is unavailable")
}
return stat.Uid, nil
}
// New returns the HTTP verifier. It has no network listener and is safe to use
// with an httptest server only for synthetic test registries.
func New(store *registry.Store, logger *log.Logger, now func() time.Time) http.Handler {
@@ -101,7 +113,7 @@ func ListenAndServe(ctx context.Context, config Config) error {
if err := validateSocketParent(config.Socket); err != nil {
return err
}
store, err := registry.Open(config.RegistryRoot)
store, err := registry.OpenReadOnly(config.RegistryRoot)
if err != nil {
return err
}
@@ -164,6 +176,20 @@ func validateSocketParent(socket string) error {
if !info.IsDir() || info.Mode()&os.ModeSymlink != 0 {
return errors.New("socket parent is not a directory")
}
if info.Mode().Perm()&0o022 != 0 {
return errors.New("socket parent is group or world writable")
}
stat, ok := info.Sys().(*syscall.Stat_t)
if !ok {
return errors.New("socket parent owner is unavailable")
}
owner, err := socketParentOwner(parent)
if err != nil {
return err
}
if stat.Uid != uint32(os.Geteuid()) || owner != stat.Uid {
return errors.New("socket parent owner mismatch")
}
resolved, err := filepath.EvalSymlinks(parent)
if err != nil || resolved != parent {
return errors.New("socket parent is not canonical")
@@ -212,6 +212,7 @@ func TestVerifyMapsMultipleLegacyRecordsToServiceUnavailable(t *testing.T) {
func TestListenAndServeFailsClosedForInvalidPathsAndCollisions(t *testing.T) {
root := t.TempDir()
initializeRegistry(t, root)
parent := t.TempDir()
for _, tc := range []struct {
name, registryRoot, socket string
@@ -252,8 +253,47 @@ func TestListenAndServeFailsClosedForInvalidPathsAndCollisions(t *testing.T) {
}
}
func TestListenAndServeRejectsUnsafeSocketParents(t *testing.T) {
root := t.TempDir()
initializeRegistry(t, root)
for _, mode := range []os.FileMode{0o770, 0o702} {
t.Run("mode "+mode.String(), func(t *testing.T) {
parent := t.TempDir()
if err := os.Chmod(parent, mode); err != nil {
t.Fatalf("Chmod(%v) error = %v", mode, err)
}
socket := filepath.Join(parent, "verify.sock")
err := ListenAndServe(context.Background(), Config{RegistryRoot: root, Socket: socket, Logger: log.New(&bytes.Buffer{}, "", 0), Now: time.Now})
if err == nil {
t.Fatalf("ListenAndServe(parent mode %v) error = nil, want refusal", mode)
}
if _, err := os.Lstat(socket); !errors.Is(err, os.ErrNotExist) {
t.Fatalf("unsafe parent created socket: %v", err)
}
})
}
}
func TestListenAndServeRejectsForeignOwnedSocketParent(t *testing.T) {
root := t.TempDir()
initializeRegistry(t, root)
parent := t.TempDir()
socket := filepath.Join(parent, "verify.sock")
previous := socketParentOwner
t.Cleanup(func() { socketParentOwner = previous })
socketParentOwner = func(string) (uint32, error) { return uint32(os.Geteuid()) + 1, nil }
err := ListenAndServe(context.Background(), Config{RegistryRoot: root, Socket: socket, Logger: log.New(&bytes.Buffer{}, "", 0), Now: time.Now})
if err == nil {
t.Fatal("ListenAndServe() error = nil, want foreign-parent refusal")
}
if _, err := os.Lstat(socket); !errors.Is(err, os.ErrNotExist) {
t.Fatalf("foreign parent created socket: %v", err)
}
}
func TestListenAndServeReclaimsOnlyOwnedStaleSocketAndCleansUpOnCancellation(t *testing.T) {
root := t.TempDir()
initializeRegistry(t, root)
socket := filepath.Join(t.TempDir(), "verify.sock")
stale, err := net.ListenUnix("unix", &net.UnixAddr{Name: socket, Net: "unix"})
if err != nil {
@@ -290,6 +330,7 @@ func TestListenAndServeReclaimsOnlyOwnedStaleSocketAndCleansUpOnCancellation(t *
func TestListenAndServeRefusesStaleSocketWithForeignOwner(t *testing.T) {
root := t.TempDir()
initializeRegistry(t, root)
socket := filepath.Join(t.TempDir(), "verify.sock")
stale, err := net.ListenUnix("unix", &net.UnixAddr{Name: socket, Net: "unix"})
if err != nil {
@@ -313,6 +354,7 @@ func TestListenAndServeRefusesStaleSocketWithForeignOwner(t *testing.T) {
func TestListenAndServeRefusesLiveSocket(t *testing.T) {
root := t.TempDir()
initializeRegistry(t, root)
socket := filepath.Join(t.TempDir(), "verify.sock")
live, err := net.ListenUnix("unix", &net.UnixAddr{Name: socket, Net: "unix"})
if err != nil {
@@ -330,6 +372,7 @@ func TestListenAndServeRefusesLiveSocket(t *testing.T) {
func TestListenAndServeCleanupRefusesChangedSocketPath(t *testing.T) {
root := t.TempDir()
initializeRegistry(t, root)
socket := filepath.Join(t.TempDir(), "verify.sock")
ctx, cancel := context.WithCancel(context.Background())
errs := make(chan error, 1)
@@ -363,6 +406,17 @@ func TestListenAndServeCleanupRefusesChangedSocketPath(t *testing.T) {
}
}
func initializeRegistry(t *testing.T, root string) {
t.Helper()
store, err := registry.Open(root)
if err != nil {
t.Fatalf("Open() error = %v", err)
}
if err := store.Close(); err != nil {
t.Fatalf("Close() error = %v", err)
}
}
func waitForReadySocket(t *testing.T, socket string) {
t.Helper()
deadline := time.Now().Add(2 * time.Second)