From 134dc1977c6e5c6d214f72e42042279e095f60fb Mon Sep 17 00:00:00 2001 From: User Date: Fri, 21 Aug 2026 02:14:50 +0200 Subject: [PATCH] fix: run DWH verification with read-only registry access --- tools/dwh-auth/internal/registry/store.go | 66 ++++++++++++++-- .../dwh-auth/internal/registry/store_test.go | 76 +++++++++++++++++++ .../internal/securefile/securefile_linux.go | 38 ++++++++-- .../securefile/securefile_linux_test.go | 43 +++++++++++ tools/dwh-auth/internal/service/service.go | 28 ++++++- .../dwh-auth/internal/service/service_test.go | 54 +++++++++++++ 6 files changed, 291 insertions(+), 14 deletions(-) diff --git a/tools/dwh-auth/internal/registry/store.go b/tools/dwh-auth/internal/registry/store.go index 832c9fb8..64e5ff1a 100644 --- a/tools/dwh-auth/internal/registry/store.go +++ b/tools/dwh-auth/internal/registry/store.go @@ -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 } diff --git a/tools/dwh-auth/internal/registry/store_test.go b/tools/dwh-auth/internal/registry/store_test.go index 058cd8bd..436da322 100644 --- a/tools/dwh-auth/internal/registry/store_test.go +++ b/tools/dwh-auth/internal/registry/store_test.go @@ -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() diff --git a/tools/dwh-auth/internal/securefile/securefile_linux.go b/tools/dwh-auth/internal/securefile/securefile_linux.go index 2783ed6e..c3080945 100644 --- a/tools/dwh-auth/internal/securefile/securefile_linux.go +++ b/tools/dwh-auth/internal/securefile/securefile_linux.go @@ -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 diff --git a/tools/dwh-auth/internal/securefile/securefile_linux_test.go b/tools/dwh-auth/internal/securefile/securefile_linux_test.go index 7f85a9a9..38c4e664 100644 --- a/tools/dwh-auth/internal/securefile/securefile_linux_test.go +++ b/tools/dwh-auth/internal/securefile/securefile_linux_test.go @@ -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) diff --git a/tools/dwh-auth/internal/service/service.go b/tools/dwh-auth/internal/service/service.go index 73b1b9f7..c8fa5c80 100644 --- a/tools/dwh-auth/internal/service/service.go +++ b/tools/dwh-auth/internal/service/service.go @@ -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") diff --git a/tools/dwh-auth/internal/service/service_test.go b/tools/dwh-auth/internal/service/service_test.go index f963a415..42ecc834 100644 --- a/tools/dwh-auth/internal/service/service_test.go +++ b/tools/dwh-auth/internal/service/service_test.go @@ -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)