diff --git a/.github/workflows/deployment.yml b/.github/workflows/deployment.yml index aa0611f0..ff35a0d2 100644 --- a/.github/workflows/deployment.yml +++ b/.github/workflows/deployment.yml @@ -147,6 +147,9 @@ jobs: with: go-version: "1.26.5" cache-dependency-path: tools/tht/go.sum + - name: Run native Windows retained-capability tests + working-directory: tools/tht + run: go test ./internal/safeio ./internal/backup -count=1 - name: Verify Windows clone contract shell: pwsh run: ./scripts/test-windows-clone-contract.ps1 diff --git a/tools/tht/internal/backup/preflight.go b/tools/tht/internal/backup/preflight.go index 00b0203a..49041f97 100644 --- a/tools/tht/internal/backup/preflight.go +++ b/tools/tht/internal/backup/preflight.go @@ -5,6 +5,7 @@ import ( "archive/zip" "bytes" "context" + "crypto/rand" "crypto/sha256" "encoding/binary" "encoding/hex" @@ -25,6 +26,14 @@ import ( var archiveDrivePath = regexp.MustCompile(`^[A-Za-z]:/`) +func newStagingArchiveName() (string, error) { + value := make([]byte, 16) + if _, err := rand.Read(value); err != nil { + return "", err + } + return "archive-" + hex.EncodeToString(value) + ".zip", nil +} + // PreflightRequest identifies an archive and the explicit protections required to inspect a // restore that contains external secret payloads. Preflight never writes to the installation. type PreflightRequest struct { @@ -97,9 +106,10 @@ type verifiedArchive struct { // stagedArchive holds an installation-private, immutable copy of the exact bytes accepted by // Preflight. The original archive remains retained only for provenance revalidation. type stagedArchive struct { - file *os.File - path string - directory string + file *os.File + parent safeio.PrivateDirectoryHandle + name string + path string } type inspectedArchiveEntry struct { @@ -278,21 +288,44 @@ func (result PreflightResult) StageArchive(ctx context.Context) (_ *stagedArchiv if result.ArchiveSize < 0 || freeBytes < uint64(result.ArchiveSize) { return nil, errors.New("insufficient free disk space for private restore staging archive") } - directory, err := os.MkdirTemp(result.stagingRoot, "archive-") - if err != nil { - return nil, errors.New("create private restore staging directory") + parent, found, err := safeio.OpenPrivateDirectory(result.stagingRoot, true) + if err != nil || !found { + if parent != nil { + _ = parent.Close() + } + return nil, errors.New("open private restore staging root") } - if err := safeio.ProtectPrivateDirectory(directory); err != nil { - _ = os.Remove(directory) - return nil, errors.New("protect private restore staging directory") + var ( + file *os.File + name string + ) + for attempt := 0; attempt < 8; attempt++ { + name, err = newStagingArchiveName() + if err != nil { + _ = parent.Close() + return nil, errors.New("create private restore staging archive") + } + var created bool + file, created, err = parent.CreateRegularFile(name) + if err != nil { + _ = parent.Close() + return nil, errors.New("create private restore staging archive") + } + if created { + break + } + file = nil } - path := filepath.Join(directory, "archive.zip") - file, err := safeio.CreateCanonicalNewPrivateFile(path) - if err != nil { - _ = os.Remove(directory) + if file == nil { + _ = parent.Close() return nil, errors.New("create private restore staging archive") } - staged := &stagedArchive{file: file, path: path, directory: directory} + staged := &stagedArchive{ + file: file, + parent: parent, + name: name, + path: filepath.Join(result.stagingRoot, name), + } completed := false defer func() { if !completed { @@ -343,7 +376,7 @@ func (result PreflightResult) StageArchive(ctx context.Context) (_ *stagedArchiv return staged, nil } -// Close removes only the staging file and directory created by StageArchive. +// Close removes only the staging file created by StageArchive through its retained directory. func (staged *stagedArchive) Close() error { if staged == nil { return nil @@ -355,18 +388,19 @@ func (staged *stagedArchive) Close() error { } staged.file = nil } - if staged.path != "" { - if err := os.Remove(staged.path); err != nil && !errors.Is(err, os.ErrNotExist) { + if staged.parent != nil { + safeio.NotifyPrivateDirectoryTestHookForTest("before-stage-archive-remove") + removed, err := staged.parent.RemoveRegular(staged.name) + if err != nil || !removed { failed = true } - staged.path = "" - } - if staged.directory != "" { - if err := os.Remove(staged.directory); err != nil && !errors.Is(err, os.ErrNotExist) { + if err := staged.parent.Close(); err != nil { failed = true } - staged.directory = "" + staged.parent = nil } + staged.name = "" + staged.path = "" if failed { return errors.New("destroy private restore staging archive") } diff --git a/tools/tht/internal/backup/preflight_test.go b/tools/tht/internal/backup/preflight_test.go index ae9c58dc..76a1dcc1 100644 --- a/tools/tht/internal/backup/preflight_test.go +++ b/tools/tht/internal/backup/preflight_test.go @@ -95,8 +95,8 @@ func TestPreflightStagesOnTheAccountedInstallationFilesystemInsteadOfTMPDIR(t *t t.Fatalf("free-space targets = %q, want installation control directory %q", capacityTargets, installation.ControlDirectory()) } stagingRoot := filepath.Join(installation.ControlDirectory(), "restore-staging") - if relative, err := filepath.Rel(stagingRoot, staged.directory); err != nil || relative == "." || strings.HasPrefix(relative, "..") { - t.Fatalf("staging directory = %q, want a child of %q", staged.directory, stagingRoot) + if filepath.Dir(staged.path) != result.stagingRoot { + t.Fatalf("staging archive directory = %q, want direct child directory %q", filepath.Dir(staged.path), stagingRoot) } } diff --git a/tools/tht/internal/backup/preflight_unix_test.go b/tools/tht/internal/backup/preflight_unix_test.go index ce691fe7..f843f20e 100644 --- a/tools/tht/internal/backup/preflight_unix_test.go +++ b/tools/tht/internal/backup/preflight_unix_test.go @@ -4,6 +4,7 @@ package backup import ( "context" + "errors" "os" "path/filepath" "strings" @@ -79,3 +80,78 @@ func TestStageArchiveCreatesUnixPrivateRegularFile(t *testing.T) { t.Fatalf("staged archive mode = %#o, want 0600", got) } } + +func TestStageArchiveCloseUsesPinnedRootAfterAncestorSwap(t *testing.T) { + installation := preflightTestInstallation(t) + if err := os.MkdirAll(installation.ControlDirectory(), 0o700); err != nil { + t.Fatal(err) + } + archive := filepath.Join(t.TempDir(), "valid.zip") + writePreflightArchive(t, archive, preflightArchiveSpec{ + entries: []preflightArchiveEntry{{path: "configuration/operator.env", body: []byte("safe")}}, + }) + result, err := Preflight(context.Background(), installation, PreflightRequest{Archive: archive, Confirm: true}, permissivePreflightDependencies()) + if err != nil { + t.Fatal(err) + } + defer result.CloseArchive() + + outsideParent, err := filepath.EvalSymlinks(t.TempDir()) + if err != nil { + t.Fatal(err) + } + outside := filepath.Join(outsideParent, "outside") + if err := os.Mkdir(outside, 0o700); err != nil { + t.Fatal(err) + } + if err := safeio.ProtectPrivateDirectory(outside); err != nil { + t.Fatal(err) + } + sentinelPath := filepath.Join(outside, "sentinel") + sentinel := []byte("outside sentinel") + if err := os.WriteFile(sentinelPath, sentinel, 0o600); err != nil { + t.Fatal(err) + } + + swapped := false + restoreHook := safeio.SetPrivateDirectoryTestHookForTest(func(stage string) { + if stage != "before-stage-archive-remove" || swapped { + return + } + swapped = true + movedRoot := result.stagingRoot + "-original" + if err := os.Rename(result.stagingRoot, movedRoot); err != nil { + t.Fatal(err) + } + if err := os.Symlink(outside, result.stagingRoot); err != nil { + t.Fatal(err) + } + }) + defer restoreHook() + + staged, err := result.StageArchive(context.Background()) + if err != nil { + t.Fatal(err) + } + stagedName := staged.name + movedRoot := result.stagingRoot + "-original" + if filepath.Dir(staged.path) != result.stagingRoot { + t.Fatalf("staged archive directory = %q, want %q", filepath.Dir(staged.path), result.stagingRoot) + } + if err := staged.Close(); err != nil { + t.Fatal(err) + } + if !swapped { + t.Fatal("stage archive cleanup hook did not run") + } + if _, err := os.Stat(filepath.Join(movedRoot, stagedName)); !errors.Is(err, os.ErrNotExist) { + t.Fatalf("moved staging archive = %v, want os.ErrNotExist", err) + } + gotSentinel, err := os.ReadFile(sentinelPath) + if err != nil { + t.Fatal(err) + } + if string(gotSentinel) != string(sentinel) { + t.Fatalf("outside sentinel = %q, want %q", gotSentinel, sentinel) + } +} diff --git a/tools/tht/internal/backup/preflight_windows_test.go b/tools/tht/internal/backup/preflight_windows_test.go index 5968e83b..7feec99d 100644 --- a/tools/tht/internal/backup/preflight_windows_test.go +++ b/tools/tht/internal/backup/preflight_windows_test.go @@ -4,6 +4,7 @@ package backup import ( "context" + "errors" "os" "path/filepath" "testing" @@ -34,10 +35,77 @@ func TestStageArchiveProtectsWindowsStagingArtifactsWithOwnerOnlyACLs(t *testing if err := safeio.ValidatePrivateDirectory(result.stagingRoot); err != nil { t.Fatalf("staging root ACL = %v, want owner-only", err) } - if err := safeio.ValidatePrivateDirectory(staged.directory); err != nil { - t.Fatalf("staged archive directory ACL = %v, want owner-only", err) - } if err := safeio.ValidatePrivateRegular(staged.path); err != nil { t.Fatalf("staged archive ACL = %v, want owner-only", err) } } + +func TestStageArchiveCloseUsesPinnedRootAfterAncestorSwap(t *testing.T) { + installation := preflightTestInstallation(t) + if err := os.MkdirAll(installation.ControlDirectory(), 0o700); err != nil { + t.Fatal(err) + } + archive := filepath.Join(t.TempDir(), "valid.zip") + writePreflightArchive(t, archive, preflightArchiveSpec{ + entries: []preflightArchiveEntry{{path: "configuration/operator.env", body: []byte("safe")}}, + }) + result, err := Preflight(context.Background(), installation, PreflightRequest{Archive: archive, Confirm: true}, permissivePreflightDependencies()) + if err != nil { + t.Fatal(err) + } + defer result.CloseArchive() + + outsideParent, err := filepath.EvalSymlinks(t.TempDir()) + if err != nil { + t.Fatal(err) + } + outside := filepath.Join(outsideParent, "outside") + if err := os.Mkdir(outside, 0o700); err != nil { + t.Fatal(err) + } + if err := safeio.ProtectPrivateDirectory(outside); err != nil { + t.Fatal(err) + } + sentinelPath := filepath.Join(outside, "sentinel") + sentinel := []byte("outside sentinel") + if err := os.WriteFile(sentinelPath, sentinel, 0o600); err != nil { + t.Fatal(err) + } + + attemptedSwap := false + restoreHook := safeio.SetPrivateDirectoryTestHookForTest(func(stage string) { + if stage != "before-stage-archive-remove" || attemptedSwap { + return + } + attemptedSwap = true + if err := os.Rename(result.stagingRoot, result.stagingRoot+"-moved"); err == nil { + t.Fatal("staging-root rename succeeded while Close retained its directory handle") + } + }) + defer restoreHook() + + staged, err := result.StageArchive(context.Background()) + if err != nil { + t.Fatal(err) + } + stagedName := staged.name + if filepath.Dir(staged.path) != result.stagingRoot { + t.Fatalf("staged archive directory = %q, want %q", filepath.Dir(staged.path), result.stagingRoot) + } + if err := staged.Close(); err != nil { + t.Fatal(err) + } + if !attemptedSwap { + t.Fatal("stage archive cleanup hook did not run") + } + if _, err := os.Stat(filepath.Join(result.stagingRoot, stagedName)); !errors.Is(err, os.ErrNotExist) { + t.Fatalf("staging archive = %v, want os.ErrNotExist", err) + } + gotSentinel, err := os.ReadFile(sentinelPath) + if err != nil { + t.Fatal(err) + } + if string(gotSentinel) != string(sentinel) { + t.Fatalf("outside sentinel = %q, want %q", gotSentinel, sentinel) + } +} diff --git a/tools/tht/internal/safeio/files_test.go b/tools/tht/internal/safeio/files_test.go index 3bcd3efa..dd76b196 100644 --- a/tools/tht/internal/safeio/files_test.go +++ b/tools/tht/internal/safeio/files_test.go @@ -149,6 +149,68 @@ func TestCreateCanonicalNewPrivateFileProvidesPrivateStreamingWriter(t *testing. } } +func TestPrivateDirectoryCreatesAndRemovesStreamingRegularFile(t *testing.T) { + temporaryRoot, err := filepath.EvalSymlinks(os.TempDir()) + if err != nil { + t.Fatal(err) + } + root, err := os.MkdirTemp(temporaryRoot, "tht-safeio-stream-") + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { _ = os.RemoveAll(root) }) + if err := ProtectPrivateDirectory(root); err != nil { + t.Fatal(err) + } + directory, found, err := OpenPrivateDirectory(root, true) + if err != nil || !found { + t.Fatalf("OpenPrivateDirectory() = found %v, err %v", found, err) + } + defer directory.Close() + + file, created, err := directory.CreateRegularFile("archive-stream.zip") + if err != nil || !created || file == nil { + t.Fatalf("CreateRegularFile() = file %v, created %v, err %v", file, created, err) + } + if _, err := file.Write([]byte("private archive")); err != nil { + t.Fatal(err) + } + if err := file.Sync(); err != nil { + t.Fatal(err) + } + if _, err := file.Seek(0, io.SeekStart); err != nil { + t.Fatal(err) + } + streamed := make([]byte, len("private archive")) + if _, err := io.ReadFull(file, streamed); err != nil { + t.Fatal(err) + } + if string(streamed) != "private archive" { + t.Fatalf("streamed contents = %q, want %q", streamed, "private archive") + } + if err := file.Close(); err != nil { + t.Fatal(err) + } + removed, err := directory.RemoveRegular("archive-stream.zip") + if err != nil || !removed { + t.Fatalf("RemoveRegular() = removed %v, err %v", removed, err) + } + + if created, err := directory.CreateRegular("existing.zip", []byte("existing")); err != nil || !created { + t.Fatalf("CreateRegular(existing.zip) = created %v, err %v", created, err) + } + if file, created, err := directory.CreateRegularFile("existing.zip"); err != nil || created || file != nil { + t.Fatalf("CreateRegularFile(existing.zip) = file %v, created %v, err %v", file, created, err) + } + if created, err := directory.CreateRegular("target.zip", []byte("target")); err != nil || !created { + t.Fatalf("CreateRegular(target.zip) = created %v, err %v", created, err) + } + testsupport.SymlinkOrSkip(t, filepath.Join(root, "target.zip"), filepath.Join(root, "linked.zip")) + if file, created, err := directory.CreateRegularFile("linked.zip"); !errors.Is(err, ErrUnsafeFile) || created || file != nil { + t.Fatalf("CreateRegularFile(linked.zip) = file %v, created %v, err %v", file, created, err) + } +} + func TestPreflightPrivateDirectoryAllowsOnlyAMissingFinalComponentWithoutMutation(t *testing.T) { temporaryRoot, err := filepath.EvalSymlinks(os.TempDir()) if err != nil { diff --git a/tools/tht/internal/safeio/private_root.go b/tools/tht/internal/safeio/private_root.go index aac603f4..855814ca 100644 --- a/tools/tht/internal/safeio/private_root.go +++ b/tools/tht/internal/safeio/private_root.go @@ -1,6 +1,7 @@ package safeio import ( + "os" "strings" "sync" ) @@ -13,6 +14,7 @@ type PrivateDirectoryHandle interface { Validate() error OpenChild(name string, ensure bool) (PrivateDirectoryHandle, bool, error) CreateRegular(name string, contents []byte) (bool, error) + CreateRegularFile(name string) (*os.File, bool, error) ReadRegular(name string, maximum int64) ([]byte, bool, error) ReplaceRegular(name string, contents []byte) error RemoveRegular(name string) (bool, error) diff --git a/tools/tht/internal/safeio/private_root_unix.go b/tools/tht/internal/safeio/private_root_unix.go index 532c7c39..fcf0161c 100644 --- a/tools/tht/internal/safeio/private_root_unix.go +++ b/tools/tht/internal/safeio/private_root_unix.go @@ -201,6 +201,48 @@ func (directory *unixPrivateDirectory) CreateRegular(name string, contents []byt return true, nil } +func (directory *unixPrivateDirectory) CreateRegularFile(name string) (*os.File, bool, error) { + if directory.Validate() != nil || !validPrivateLeafName(name) { + return nil, false, ErrUnsafeFile + } + descriptor, err := unix.Openat(directory.descriptor, name, + unix.O_RDWR|unix.O_CREAT|unix.O_EXCL|unix.O_CLOEXEC|unix.O_NOFOLLOW, 0o600) + if errors.Is(err, unix.EEXIST) { + if _, existingErr := requirePrivateUnixRootRegularAt(directory.descriptor, name, 1); existingErr != nil || directory.Validate() != nil { + return nil, false, ErrUnsafeFile + } + return nil, false, nil + } + if err != nil { + return nil, false, ErrUnsafeFile + } + file := os.NewFile(uintptr(descriptor), "tht-safeio-private-root-stream") + if file == nil { + _ = unix.Close(descriptor) + _ = unix.Unlinkat(directory.descriptor, name, 0) + return nil, false, ErrUnsafeFile + } + failed := true + defer func() { + if failed { + _ = file.Close() + _ = unix.Unlinkat(directory.descriptor, name, 0) + } + }() + if unix.Fchmod(descriptor, 0o600) != nil { + return nil, false, ErrUnsafeFile + } + var stat unix.Stat_t + if unix.Fstat(descriptor, &stat) != nil || !privateUnixRootRegular(&stat, 1) { + return nil, false, ErrUnsafeFile + } + if directory.Validate() != nil || unix.Fsync(directory.descriptor) != nil { + return nil, false, ErrUnsafeFile + } + failed = false + return file, true, nil +} + func (directory *unixPrivateDirectory) ReadRegular(name string, maximum int64) ([]byte, bool, error) { if directory.Validate() != nil || !validPrivateLeafName(name) || maximum < 0 || maximum == int64(^uint64(0)>>1) { return nil, false, ErrUnsafeFile diff --git a/tools/tht/internal/safeio/private_root_windows.go b/tools/tht/internal/safeio/private_root_windows.go index 0a3181b7..f3424adb 100644 --- a/tools/tht/internal/safeio/private_root_windows.go +++ b/tools/tht/internal/safeio/private_root_windows.go @@ -538,6 +538,41 @@ func (directory *windowsPrivateDirectory) CreateRegular(name string, contents [] return true, nil } +func (directory *windowsPrivateDirectory) CreateRegularFile(name string) (*os.File, bool, error) { + if directory.Validate() != nil || !validPrivateLeafName(name) { + return nil, false, ErrUnsafeFile + } + value, err := createWindowsPrivateRegularAt(directory.handle, name) + if err != nil { + existing, existingErr := openWindowsPrivateRegularAt(directory.handle, name, windows.FILE_GENERIC_READ, 1) + if existingErr == nil { + if existing.Close() != nil || directory.Validate() != nil { + return nil, false, ErrUnsafeFile + } + return nil, false, nil + } + return nil, false, ErrUnsafeFile + } + failed := true + defer func() { + if failed { + _ = closeAndDeleteWindowsPrivateRegular(value) + } + }() + info, err := privateWindowsRegularInfo(value.handle, 1) + if err != nil || directory.Validate() != nil { + return nil, false, ErrUnsafeFile + } + value.info = info + file := os.NewFile(uintptr(value.handle), "tht-safeio-private-root-stream") + if file == nil { + return nil, false, ErrUnsafeFile + } + value.handle = 0 + failed = false + return file, true, nil +} + func (directory *windowsPrivateDirectory) ReadRegular(name string, maximum int64) ([]byte, bool, error) { if directory.Validate() != nil || !validPrivateLeafName(name) || maximum < 0 || maximum == int64(^uint64(0)>>1) { return nil, false, ErrUnsafeFile